Say why triage acknowledge writes nothing, instead of promising it will

[yh928]

Summary

The Acknowledge arm of triage::escalation::apply_decision logged memory-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:

	where	when
what the trigger was	trigger_history daily JSONL	written before the triage gates, so it survives even with triage disabled
what it was judged to be	DomainEvent::TriggerEvaluated	published for every action, a few lines above this arm

Neither 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

• The comment now states why the arm writes nothing, and names the two records that already exist.
• The log line states what happened and where to look, and carries source and card_linked alongside the existing fields.
• The sibling DROP arm gets the same two fields. Both are "no downstream work" verdicts; a dashboard filtering on source= would otherwise silently see only half of them.
• The genuinely-missing piece — a record of what happened after the verdict, for every action rather than this one — is filed as Record what a trigger's verdict actually led to #5408 and referenced from the comment, so the next reader finds the work instead of re-deriving the note.

No behaviour change: the arm wrote nothing before and writes nothing now.

Acceptance criteria

• The misleading note is gone — replaced by the reason, not by silence.
• The reason is checkable — the comment names trigger_history and TriggerEvaluated, both verifiable in the same file's call path.
• The follow-up is findable — Record what a trigger's verdict actually led to #5408 is linked from the code, not only from this PR.
• Sibling arms stay greppable together — DROP and ACKNOWLEDGE carry the same field set.
• Tests pass — agent::triage 70 tests; cargo check --lib --all-features clean.

Related

• Record what a trigger's verdict actually led to #5408 — record what a trigger's verdict actually led to (what the removed note was reaching for).
• Surface background work in the app so a user can watch it and take it over #5409 — surface background work in the app; renders Record what a trigger's verdict actually led to #5408.
• Internal agents store their own prompts as the user's conversation memories #5312 / fix(memory): stop copying the conversation into memory; let recall answer with memories #5315 — why writing the input to memory a second time is the wrong move.

Summary by CodeRabbit

• New Features
  • Improved drop and acknowledgment activity logs with trigger source and task-card details.
  • Clarified acknowledgment records to show whether trigger history is retained or only the evaluation result is preserved.
  • Indicated when no memory entry is created, including distinct retention behavior for Composio, Webhook, Cron, and Webview Integration triggers.

[greptile-apps]

yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 7137a70f-5c2a-4aaf-b7be-190aaf093458

📥 Commits

Reviewing files that changed from the base of the PR and between 8e65c40 and 0fa822b.

📒 Files selected for processing (2)

• src/openhuman/agent/triage/escalation.rs
• src/openhuman/agent/triage/escalation_tests.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/openhuman/agent/triage/escalation.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

The escalation logic updates drop and acknowledge logs with trigger source and task-card linkage. Acknowledge handling reports source-specific input retention, performs no memory write, and records evaluation through TriggerEvaluated.

Changes

Triage decision logging

Layer / File(s)	Summary
Drop and acknowledge decision logging src/openhuman/agent/triage/escalation.rs, src/openhuman/agent/triage/escalation_tests.rs	Drop and acknowledge logs include trigger source and task-card linkage. Acknowledge handling documents no memory write and reports Composio trigger-history retention or verdict-only retention for other sources. Tests cover all supported sources.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 0fa82

This localized change clarifies triage acknowledgement and drop logging without changing behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: senamakel

Poem

I’m a rabbit, logging hops with care,
Sources and task cards now join the air.
No memory write when acknowledge is near,
Retention notes make records clear.
TriggerEvaluated marks the way,
Drop and ack logs shine today.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: updating triage acknowledge logging to explain that it writes nothing instead of implying a future write.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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

📥 Commits

Reviewing files that changed from the base of the PR and between d9d03af and e0cd845.

📒 Files selected for processing (1)

• src/openhuman/agent/triage/escalation.rs

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[tinysweeper]

tinysweeper found nothing blocking. Approving.

             $0.0021 · 16,131 in / 5,221 out · 3,567 cached (22%) · z-ai/glm-5.2
critique:    $0.0010 · 5,484 in  / 3,168 out · 896 cached (16%)   · z-ai/glm-5.2
security:    $0.0001 · 3,338 in  / 24 out    · 2,671 cached (80%) · z-ai/glm-5.2
tests:       $0.0005 · 3,253 in  / 1,044 out · 0 cached (0%)      · z-ai/glm-5.2
description: $0.0005 · 4,056 in  / 985 out   · 0 cached (0%)      · z-ai/glm-5.2

[coderabbitai]

🧹 Nitpick comments (1)

src/openhuman/agent/triage/escalation.rs (1)

799-813: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover TriggerSource::External in this regression test.

TriggerSource supports External, but this loop covers only Composio, webhook, cron, and webview. Add an External case and assert none — verdict only. The current wildcard returns the expected value, but the test does not protect this supported source from a future regression.

Proposed test addition

             TriggerSource::WebviewIntegration {
                 provider: "gmail".into(),
                 account_id: "a".into(),
             },
+            TriggerSource::External {
+                caller_id: "caller".into(),
+                reason: "reason".into(),
+            },

🤖 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 799 - 813, Add a
TriggerSource::External case to the regression-test loop alongside the existing
source variants, using the appropriate External fields, and assert that it
produces no action (“none”) with only the verdict. Keep the existing assertions
and wildcard behavior unchanged.

🤖 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.

Nitpick comments:
In `@src/openhuman/agent/triage/escalation.rs`:
- Around line 799-813: Add a TriggerSource::External case to the regression-test
loop alongside the existing source variants, using the appropriate External
fields, and assert that it produces no action (“none”) with only the verdict.
Keep the existing assertions and wildcard behavior unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 141361fd-33d2-4dc7-bada-666e850eb122

📥 Commits

Reviewing files that changed from the base of the PR and between 8774fe4 and a994ca3.

📒 Files selected for processing (1)

• src/openhuman/agent/triage/escalation.rs

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[M3gA-Mind]

Maintainer review pass. The change is worth landing — replacing a note that described a wrong plan with the reason the arm writes nothing, and the argument for why the planned memory-write would have duplicated a synced document, are both right.

Two things are blocking, and the first is better news than it looks.

The five red checks are not yours

I read all five job logs. Not one of them fails in code this PR touches:

check	actual failure
Rust Quality (fmt, clippy)	rustfmt diff in src/openhuman/web3/wallet/chains/btc.rs:464
Rust Feature-Gate Smoke	error[E0432]: unresolved import in src/openhuman/memory/diff/stub.rs:29 and diff/mod.rs:79
Rust RSS Benchmark	the same two E0432s
Markdown Link Check	20 × [403] on producthunt.com badge URLs in docs/README.*.md
PR CI Gate	cascade from Rust Quality

That run is from 11 Aug against a main that was broken at the time — the log still shows vendor/tinyplace, which has since been removed entirely (#5847), and src/openhuman/memory/diff/ no longer exists on main either. A rebase should clear all five without you touching anything.

A real defect: gate_linked_card_terminal lost its doc comment

retained_input_note was inserted between an existing doc block and the function it documented. On your head, escalation.rs:350-355 — "Terminally gate a card-linked trigger that triage decided to drop / acknowledge…" — now attaches to retained_input_note at line 364, which reads as a paragraph about card gating followed by an unrelated paragraph about trigger archives. gate_linked_card_terminal at line 371 is left with no doc comment at all.

Moving the new function (with its own doc block) above line 350, or below gate_linked_card_terminal, fixes it. Worth catching precisely because this PR's whole thesis is that the comments should say what actually happens.

Rebase note

Your one conflicting file has a single cause: 9a11266d5 refactor: extract inline tests into sibling files moved escalation.rs's mod tests { … } into escalation_tests.rs, so escalation.rs now ends with a #[path = "escalation_tests.rs"] mod tests; stanza. Keep that stanza and move only_composio_claims_a_retained_input into escalation_tests.rs (dedent by four; its use super::*; already reaches retained_input_note and TriggerSource). The production hunks should apply cleanly.

I have not pushed anything to your branch.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[yh928]

Rebased onto main (8e65c4008) and pushed — 0fa822b52. Both of your points are addressed.

The five red checks were, as you said, not mine. The rebase clears them without touching anything: vendor/tinyplace is gone (#5847), src/openhuman/memory/diff/ no longer exists so its two E0432s can't recur, and the btc.rs rustfmt diff is not on this head. Thanks for reading all five logs — that turned a scary-looking wall of red into a no-op.

The doc comment defect — fixed, and it was real. retained_input_note had indeed been inserted between the /// Terminally gate a card-linked trigger … block and the function it documents, leaving gate_linked_card_terminal undocumented and the gating paragraph reading as a preamble to an unrelated one. retained_input_note now sits above that block with its own doc, so each comment is adjacent to what it describes.

One thing your review predates: escalation.rs's inline mod tests moved to escalation_tests.rs on main. only_composio_claims_a_retained_input went there unchanged.

Verified locally: cargo test --lib triage::escalation — 8/8 pass. cargo fmt --check, layout gate, and the full pre-push suite (lint, tsc, clippy -D warnings on both crates, token lints) clean.

The substance is unchanged: the arm still writes nothing, the log now says what actually survives per source, and retained_input_note still refuses to claim a trigger-history archive for a webhook/cron/webview acknowledge that never got one.

[tinysweeper]

How this change flows

3 changed behaviours across 21 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 36 further behaviours left out to keep the diagram readable.

flowchart LR
  n0["apply_decision<br/>changed"]:::changed
  n1["dispatch_linked_card<br/>changed"]:::changed
  n2["...n_escalate_failure_publishes_failed_event<br/>changed"]:::changed
  n3["test_events_guard"]:::impacted
  n4["...sion_react_failure_publishes_failed_event"]:::impacted
  n5["envelope"]:::impacted
  n6["...acknowledge_gates_linked_card_to_rejected"]:::impacted
  n7["...sion_acknowledge_only_publishes_evaluated"]:::impacted
  n8["...cision_drop_gates_linked_card_to_rejected"]:::impacted
  n0 -->|calls| n1
  n2 -->|calls| n3
  n2 -->|tests| n3
  n2 -->|calls| n5
  n2 -->|tests| n5
  n4 -->|calls| n3
  n4 -->|tests| n3
  n4 -->|calls| n5
  n4 -->|tests| n5
  n6 -->|calls| n3
  n6 -->|tests| n3
  n6 -->|calls| n5
  n6 -->|tests| n5
  n7 -->|calls| n3
  n7 -->|tests| n3
  n7 -->|calls| n5
  n7 -->|tests| n5
  n8 -->|calls| n3
  n8 -->|tests| n3
  n8 -->|calls| n5
  n8 -->|tests| n5
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff

Loading

_{Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge.}