Say why triage acknowledge writes nothing, instead of promising it will - #5410
Open
yh928 wants to merge 1 commit into
Open
Say why triage acknowledge writes nothing, instead of promising it will#5410yh928 wants to merge 1 commit into
yh928 wants to merge 1 commit into
Conversation
…e day The arm logged `memory-write is a future addition`, which described an unimplemented plan rather than the behaviour — and the plan was wrong. Writing a summary here would duplicate a document the connector sync has already ingested: the same mail, a second copy, competing with extracted memories for the same recall slots. That is tinyhumansai#5312, and tinyhumansai#5315 is the fix; this arm should not reopen it. Acknowledge is a classification, not a write, and the two things worth keeping are already kept. What the trigger *was* is durable in the composio trigger-history JSONL, written before the triage gates so it survives even with triage disabled. What it was *judged to be* went out as `TriggerEvaluated` a few lines above, for every action. What is actually missing is a record of what happened *after* the verdict — for every action, not just this one — which belongs in its own surface rather than bolted onto one branch. Filed as tinyhumansai#5408, and referenced from the comment so the next reader finds the work instead of re-deriving the note. The log line now states what happened and where to look, and carries `source` and `card_linked`. The sibling DROP arm gets the same two fields: both are "no downstream work" verdicts, and a dashboard filtering on `source=` would otherwise silently see only half of them. agent::triage 70 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Contributor
📝 WalkthroughWalkthroughThe escalation logic updates drop and acknowledge logs with trigger source and task-card linkage. Acknowledge handling now states that it does not write memory and records evaluation through ChangesTriage decision logging
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/agent/triage/escalation.rs`:
- Around line 69-88: Update the acknowledge logging in apply_decision to
describe durable retention based on envelope.source. Keep the trigger-history
retention statement only for composio sources, and provide source-appropriate
wording for webhook, cron, and external envelopes while preserving the existing
structured fields and no-action behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 156d8dd5-d34c-48e4-86d9-b1f0e17a86a0
📒 Files selected for processing (1)
src/openhuman/agent/triage/escalation.rs
Comment on lines
+69
to
+88
| // Acknowledge is a classification, not a write. What the trigger | ||
| // *was* is already durable in the composio trigger-history JSONL, | ||
| // and what it was *judged to be* went out as `TriggerEvaluated` | ||
| // above. Copying a summary into the memory store on top of that | ||
| // would duplicate a document the connector sync already ingested — | ||
| // same mail, second copy, competing for the same recall slots — so | ||
| // this arm deliberately writes nothing. | ||
| // | ||
| // What is genuinely missing is not a copy of the input but a record | ||
| // of what happened *after* the verdict, for every action, not just | ||
| // this one. That belongs in a progress surface of its own rather | ||
| // than bolted onto the acknowledge branch (#5408). | ||
| tracing::info!( | ||
| source = %envelope.source.slug(), | ||
| label = %envelope.display_label, | ||
| external_id = %envelope.external_id, | ||
| card_linked = envelope.card_link.is_some(), | ||
| reason = %run.decision.reason, | ||
| "[triage::escalation] ACKNOWLEDGE — logged (memory-write is a future addition)" | ||
| "[triage::escalation] ACKNOWLEDGE — no autonomous action; \ | ||
| recorded as TriggerEvaluated, input retained in trigger history" |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Find source-specific history writers and TriggerEvaluated persistence.
rg -n -C 8 \
'trigger[-_ ]history|TriggerEvaluated|publish_evaluated|from_(composio|webhook|cron|external)|apply_decision' \
src/openhuman || trueRepository: tinyhumansai/openhuman
Length of output: 50379
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the triage module around acknowledgment and decision publishing.
ast-grep outline src/openhuman/agent/triage/escalation.rs --view expanded || true
sed -n '1,130p' src/openhuman/agent/triage/escalation.rs
# Inspect triage envelopes, TriggerEvaluated definitions, and decision application paths.
rg -n -C 6 'enum TriggerEnvelope|struct TriggerEnvelope|from_composio|from_webhook|from_cron|from_external|publish_evaluated|TriggerEvaluated|run_triage|apply_decision' src/openhuman/agent src/openhuman/skills/webhooks/ops.rs src/openhuman/skills/webhooks/bus.rsRepository: tinyhumansai/openhuman
Length of output: 50381
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Read the deterministic parts of TriggerEnvelope construction without running repo code.
sed -n '1,240p' src/openhuman/agent/triage/envelope.rs
# Search only for trigger-history archives or non-Composio persistence of raw trigger input.
rg -n -C 4 'trigger.*history|trigger-history|TriggerHistory|JsonL|archive|ComposioTriggerSubscriber|DomainEvent::ComposioTriggerReceived|publish_global|ComposioTriggerReceived|TriggerReceived|Cron|Webhook' \
src/openhuman/integrations src/openhuman/agent src/openhuman/skills src/openhuman/platform src/openhuman/core | \
sed -n '1,240p'
# Print the TriggerEvaluated publishing implementation.
sed -n '1,220p' src/openhuman/agent/triage/events.rsRepository: tinyhumansai/openhuman
Length of output: 30567
Make the acknowledge log source-specific.
apply_decision also receives webhook, cron, and external envelopes, and the only durable trigger-history archive is src/openhuman/integrations/composio/trigger_history.rs. Change this arm to state what is durable for the actual envelope.source, and only say “input retained in trigger history” for composio.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/agent/triage/escalation.rs` around lines 69 - 88, Update the
acknowledge logging in apply_decision to describe durable retention based on
envelope.source. Keep the trigger-history retention statement only for composio
sources, and provide source-appropriate wording for webhook, cron, and external
envelopes while preserving the existing structured fields and no-action
behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
Acknowledgearm oftriage::escalation::apply_decisionloggedmemory-write is a future addition. That described an unimplemented plan rather than the behaviour — and the plan was wrong. Replaces it with the reason the arm writes nothing, improves the log line, and files the work that note was actually pointing at.Problem
Three things, in order of how much they matter.
The planned write would have been a bug. Copying an acknowledged trigger's summary into the memory store duplicates a document the connector sync has already ingested — the same mail, a second copy, competing with extracted memories for the same recall slots. That is #5312; #5315 is the fix for the copies that already exist. A note inviting the next contributor to add another one is worse than no note.
The arm's real behaviour was undocumented. Acknowledge is a classification, and the two things worth keeping are already kept elsewhere:
trigger_historydaily JSONLDomainEvent::TriggerEvaluatedNeither is obvious from the arm, so "writes nothing" read as an omission rather than a decision.
The log said what was missing instead of what happened. A reader tailing logs for an acknowledged trigger got a parenthetical about future work and no statement of the outcome.
Solution
sourceandcard_linkedalongside the existing fields.DROParm gets the same two fields. Both are "no downstream work" verdicts; a dashboard filtering onsource=would otherwise silently see only half of them.No behaviour change: the arm wrote nothing before and writes nothing now.
Acceptance criteria
trigger_historyandTriggerEvaluated, both verifiable in the same file's call path.DROPandACKNOWLEDGEcarry the same field set.agent::triage70 tests;cargo check --lib --all-featuresclean.Related
Summary by CodeRabbit