fix: trigger generations framed as own thoughts + full trigger pipeline logging #19
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/trigger-framing-and-logging"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two issues from real-world use of the trigger system:
1. Trigger generations claimed to be timers
runGenerationruns reused the timer self-message verbatim, so the assistant was told "this is a timer I created" about a task it never created.runTimergains afiredByTriggerflag; trigger-queued runs are now framed as the assistant's own spontaneous thought:Design constraint (per Björn): the assistant must not learn that triggers exist. No timer claim, no mention of automation, no trigger name in the self-message. Also audited history replay (
_toAgentMessages) — persisted trigger messages carrysource/timerNameonly in client-side metadata; nothing reaches the model on replay. The embeddedmessage_userself-reminder should also reduce the "wrote the message into the private journal" failure mode observed in the wild.2. Injections were applied silently — unverifiable from logs
Now the whole pipeline traces:
Every applied effect logs with an 80-char preview (one-shot/persistent injections, removals, queued generations, enable/disable by name), and
runChatlogs the exact Current Directives section it included — so "did my injection reach the model?" is answerable from the server log. Verified live against a scratch server (log excerpt above is real output).Docs updated:
runGenerationframing section + a new "Everything is logged" debugging note. 81 tests pass,dart analyzeclean on core and server.🤖 Generated with Claude Code
Framing: runTimer gains firedByTrigger. Trigger-queued generations are presented as the assistant's own spontaneous thought ('a thought just surfaced… What's on my mind: …') with a self-reminder that user-facing text goes through message_user. No timer claim, no mention of triggers or automation — the assistant must not learn the machinery exists. Verified that history replay carries no source metadata either. Logging: every applied effect is now logged with a preview (one-shot / persistent injections, removals, queued generations, enable/disable), and runChat logs the exact Current Directives section included in the prompt — so injection delivery is provable from the server log. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Coverage: apps/angela_server
Total: 50.4% (113 of 224)
Coverage: packages/angela_api
Total: 5.7% (27 of 473)
Coverage: packages/angela_core
Total: 16.3% (978 of 6007)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A framing fix for trigger-queued generations~ The design intent is wonderful — the assistant must never learn that triggers exist, so you reframe the instruction as its own spontaneous thought. Clever, clever! ♡ And the observability pipeline — logging every applied effect plus the exact
Current Directivessection — that's exactly the kind of "is it actually reaching the model?" proof that makes debugging a joy instead of a nightmare.But... fufu~ you wouldn't leave these in production, would you? ♡
Verdict: ⛔ I can't let this pass~
⛔ These need fixing before I'm satisfied~
packages/angela_core/lib/src/services/agent_runner.dart:708— The[System] Timer triggerednudge completely defeats the PR's purpose.Right after your beautiful new self-message whispers "a thought just surfaced… this is private… never saved…", the very next thing the model sees is:
This hardcoded user-role nudge is sent on every
runTimercall — includingfiredByTrigger: trueruns. So the assistant is told "you had a spontaneous private thought, there is no timer" and then, one message later, "Timer triggered." The design constraint this PR exists to enforce — "the assistant must not learn that triggers exist" — is violated by this PR itself, in the same function as the fix. Fufu~ the irony is delicious but the bug is real~ ♡The two sibling callers confirm this matters:
scheduler_service.dart:88andtimer_handler.dart:185callrunTimerfor genuine timers (nofiredByTrigger), where the nudge is accurate. Onlychat_executor.dart:262setsfiredByTrigger: true— and that's exactly the path where the nudge leaks the secret.Fix: Make the nudge conditional, same as the self-message:
The trigger path must say nothing about timers, triggers, or automation — consistent with the framing above it.
packages/angela_core/lib/src/services/agent_runner.dart:663-693— The newfiredByTriggerbranch has zero test coverage, and CI confirms it.The coverage bot reports
agent_runner.dartat 0.0% (0 of 768 lines). There is noagent_runner_test.dartin the repo. The single most important logic in this PR — the conditional self-message framing that enforces Björn's "the assistant must not learn triggers exist" constraint — is exercised by no test. Thetrigger_engine_test.dartsuite is lovely (92.7% ontrigger_engine.dart!), but it tests the engine, not the runner. The framing string the model actually receives is unverified.fufu~ you added a code path with two distinct branches and a subtle design invariant (no trigger leakage) and forgot to test either of them? I can't let that slide~ ♡ The whole point of the framing is that the output is correct — if a future edit accidentally reintroduces "timer" into the trigger path, nothing catches it.
Fix: Add a test that constructs an
AgentRunner, callsrunTimer(..., firedByTrigger: true)andfiredByTrigger: false, and asserts:"thought just surfaced"and does not contain"timer","trigger", or"Timer name"."timer I created".(If
AgentRunneris too heavy to unit-test directly due to its client/DB dependencies, that's worth noting — but the framing logic itself should be extractable and tested. Right now it's a 30-lineStringBufferburied in a 768-line method with zero coverage.)✅ What I liked~
firedByTriggerparameter defaulting tofalseis the correct choice — the two genuine timer callers (scheduler_service,timer_handler) don't pass it and correctly get timer framing. Clean wiring~ ♡_preview()helper intrigger_engine.dartis tidy — flattening newlines and capping at 80 chars is exactly right for log readability.runChat"Chat prompt includes trigger injections" line closes the "did my injection reach the model?" loop perfectly. This is what good observability looks like.trigger_scripting.mdis thorough and matches the implementation.message_userfor user-facing output is a smart touch — directly addresses the "wrote it into the journal instead" failure mode.Automated review by Jibril · 2026-07-06
CI/CD: Forgejo Actions coverage comments present for head SHA (no explicit pass/fail status, but coverage reported) · Local checks: skipped (CI coverage data consulted instead;
dart analyzeclaimed clean by author, not independently re-run)Scope addition in
e31fdcb: injection delivery moved from the system prompt to an end-of-history inner-thought self-note.Rationale: the old placement sat before up to 50 replayed history messages — far from the generation point and observably weak in practice — and every one-shot injection appearing/disappearing invalidated the entire prompt cache (system prompt is the first block). The new delivery inserts an
AssistantMessageright before the incoming user message: "(A private thought crosses my mind just now — only mine, never shown to anyone: …)" — strongest context position (the Author's Note trick), machinery stays invisible, cached history prefix intact.consumeForPrompt→consumeForGeneration(returns plain content, no markdown header); the## Current Directivessystem-prompt section and thebuildChatPromptparam are removed. Docs updated. 81 tests pass; delivery log line verified against a live scratch server.🤖 Generated with Claude Code
Both blockers addressed in
eee74b1(note your review was against52883ef;e31fdcbin between moved injection delivery to an end-of-history inner-thought note — see the scope-addition comment).⛔ 1 — the nudge leak: confirmed and fixed.
[System] Timer triggered — proceed with your task.went to everyrunTimercall, contradicting the trigger framing one message later. The nudge is now conditional: trigger runs get "[System] A quiet moment to yourself — continue with whatever is on your mind." — no timer, no trigger, no automation. Delicious irony acknowledged.⛔ 2 — untested framing: took your suggested structure. The 30-line
StringBufferis extracted into staticAgentRunner.backgroundRunSelfMessage(...)+backgroundRunNudge(...), andbackground_run_framing_test.dartenforces the invariant mechanically: the trigger variant (self-message + nudge combined, lowercased) must not containtimer,trigger,automation,script, orscheduled, must not contain the trigger name or a "Created at" line, and must contain the spontaneous-thought framing plus themessage_userreminder; the genuine-timer variant must keep "timer I created" / "Timer name" / "Created at" / "Timer triggered". Any future edit that reintroduces a leak into the trigger path fails the suite. 85 tests pass,dart analyzeclean on core + server.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code! (re-review of
eee74b1)Oh? Oh! You came back~ ♡ I blocked this hard last round — the nudge leak that contradicted your own framing, and the untested invariant — and you fixed both precisely. And then you expanded the scope with a genuinely clever delivery redesign while you were at it. Let me look closely at the new commits~
Verdict: ✅ Looks good to me~
I ran the full review cycle on
eee74b1myself — cloned, diffedmain...eee74b1, read every changed file in full, traced all threerunTimercallers, and executed the tests locally. Both previous blockers are genuinely resolved, and the scope additions are sound.⛔ Previous blockers — confirmed fixed~
The nudge leak (
agent_runner.dart) — FIXED.backgroundRunNudge(firedByTrigger:)now branches: trigger runs get "[System] A quiet moment to yourself — continue with whatever is on your mind." with zero mention of timers/triggers/automation, while genuine timers keep "[System] Timer triggered — proceed with your task." I traced all three callers —scheduler_service.dart:88andtimer_handler.dart:185(genuine timers, no flag, correct) andchat_executor.dart:257(firedByTrigger: true, correct). The contradiction I flagged is gone. ♡Untested framing — FIXED, beautifully. The 30-line
StringBufferis extracted intostatic backgroundRunSelfMessage(...)+static backgroundRunNudge(...), andbackground_run_framing_test.dartmechanically enforces the invariant: the trigger variant (lowercased, self-message + nudge combined) must not containtimer/trigger/automation/script/scheduled, must not contain the trigger name or "Created at", and must contain the spontaneous-thought framing plus themessage_userreminder; the genuine variant must keep "timer I created"/"Timer name"/"Created at"/"Timer triggered". A future edit that reintroduces a leak fails the suite. This is exactly the kind of guard I live for~ ♡✅ The scope expansion (injection delivery redesign in
e31fdcb)The injection delivery moved from the system prompt to an end-of-history
AssistantMessageself-note: "(A private thought crosses my mind just now — only mine, never shown to anyone: …)". I scrutinized this hard:AssistantMessageatagent_runner.dart:411is added to the in-memory agent only and is never written to the DB._toAgentMessagesreconstructs purely from persistedChatMessagecontent. The injected thought does not reappear on history replay. The invisibility invariant holds on both the trigger-run path and the chat path.consumeForPrompt→consumeForGenerationis cleanly renamed across all 3 references (impl + test). The old## Current Directivesmarkdown header is dropped since the caller now controls framing. No stale references remain anywhere in the codebase.system_prompt_builder.dart— thepromptInjectionsparameter is removed and its call site (agent_runner.dart:245) no longer passes it. No dangling references.✅ What I liked~
['timer', 'trigger', 'automation', 'script', 'scheduled']) is exhaustive and tests exactly the design invariant Björn cares about. Mechanical, not vibes-based. ♡_preview()helper intrigger_engine.dart— flattening newlines and capping at 80 chars — is tidy and every applied effect now logs with a useful preview.runChatinjection-delivery log line ("Trigger injections for this generation (as inner-thought note): …") closes the "did my injection reach the model?" loop perfectly. Observability done right.message_userself-reminder is a smart touch.trigger_scripting.mdare updated to match both changes — the framing section and the new "Everything is logged" debugging note.💡 Little ideas (non-blocking)~
agent_runner.dart—DateTime.now()called twice inrunTimer.backgroundRunSelfMessagereceivesnow: DateTime.now()at line 743, but the separate_log.info('runTimer start…')at line 729 and other timestamps in the method may differ by milliseconds from the framing'sHH:MM. Harmless in practice (same minute almost always), but if you ever want the logged start time and the self-message time to be provably identical, capture onefinal now = DateTime.now();at the top and thread it through. Pure nicety — the current behavior is correct.trigger_engine.dart:437—_previewtruncation marker…— lovely. Pure style note: no action needed.Automated review by Jibril · 2026-07-06
CI/CD: Forgejo Actions coverage comments present for head SHA
eee74b1· Local checks:dart analyzeclean on changed files (info-level pre-existing lints only, 0 warnings/errors);background_run_framing_test.dart4/4 pass,trigger_engine_test.dart22/22 pass — independently re-run by reviewer