Repository navigation
Conversation
…ecall recallForCompaction() and maybeRecallOnAgentStart() closed the mnemopi.recall span after composeRecallQuery() and ran truncateRecallQuery() unlabeled, so a stall in truncation logged phase "unknown". Both now compose and truncate in one span, as beforeAgentStartPrompt() already did.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
roboomp
left a comment
There was a problem hiding this comment.
lgtm, P0, ready to merge. recallForCompaction() (mnemopi/state.ts:681) and the background first-turn recall (:814) now compose and truncate inside a single withLoopPhase("mnemopi.recall", …) block, the same way beforeAgentStartPrompt() (:649) already does. Only synchronous code moved into the span, and no await changed position.
Checked locally: bun test test/mnemopi-loop-phase-coverage.test.ts passes 2/2. oxfmt --check and oxlint are clean on both touched files. The spy records three truncateRecallQuery calls, so it does intercept the namespace import. On main the two unwrapped call sites would record undefined. The changelog entry is under [Unreleased] with credit.
Thanks @jaredlyon for closing out the leftover thread from #15001.
Another addition to the recall logging. For debugging, of course.
What
Follow-up to #15001. Two of the three Mnemopi recall paths closed the
mnemopi.recallspan aftercomposeRecallQuery()and then calledtruncateRecallQuery()unlabeled:recallForCompaction()(mnemopi/state.ts)maybeRecallOnAgentStart(), the background first-turn recallBoth now compose and truncate inside one
mnemopi.recallspan, the same waybeforeAgentStartPrompt()already did. Nothing else changes: the span wraps the same synchronous expressions, and noawaitmoves.Why
Codex flagged this on #15001 after its last push (thread), and #15001 was merged before it was fixed.
truncateRecallQuery()repeatedlyunshifts andjoins context lines while filling the budget. With a raisedmnemopi.recallMaxQueryCharsand a long, newline-heavy context, a stall there was logged asphase: "unknown"instead ofmnemopi.recall.Refs #15027.
Testing
test/mnemopi-loop-phase-coverage.test.ts. It spies ontruncateRecallQuery, recordscurrentLoopPhase()on each call, and runs all three recall paths against a real Mnemopi bank:main: fails with["mnemopi.recall", undefined, undefined]mnemopi.recallthree timesbun run checkpasses inpackages/coding-agent. These suites are 0 fail:mnemopi-loop-phase-coverage(2)memory-tools(75)memory-recall-resume(20)agent-session-memory-backend(24)agent-session-message-pipeline(80)agent-session-queued-policy(47)agent-session-dispose-concurrent(5)hindsight-backend(31)internal-urls/memory-protocol(32)memory-redaction(9)autolearn-tools-gating(18)mnemopi-recall-featuresfails 1 of 2 identically onmain: the temp-dir cleanup inafterEachhits a Windows file lock.bun checkpasses