fix(parser): harness-injected user lines must not split an exchange (Claude isMeta, Codex wrappers) - #168
fix(parser): harness-injected user lines must not split an exchange (Claude isMeta, Codex wrappers)#168vicnaum wants to merge 2 commits into
Conversation
…image-paste prompts lost their text)
When a Claude Code prompt includes a pasted image, the transcript has two
consecutive type:"user" lines: the real prompt ("[Image obra#1] <words>" + image
block) and an isMeta:true follow-up whose only text is
"[Image: source: /path.jpg]" (or "[Image: original WxH, displayed at ...]").
The Claude-harness loop started a new exchange on every user line and
finalizeExchange() dropped the previous one because no assistant reply sat
between them, so the real prompt was discarded and the placeholder became
user_message. Search could not find those conversations by what the user
actually asked. On one real index, 257 of 6,448 exchanges (63 transcripts)
were affected.
The same path split turns at every other harness-injected isMeta user line
(Skill tool bodies, [SYSTEM NOTIFICATION - NOT USER INPUT] task notifications,
local-command caveats, coordinator messages, "Continue from where you left
off."), e.g. a skill invocation became two exchanges with the skill body
indexed as if the user had typed it.
Fix: while an exchange is open, an injected isMeta user line neither starts a
new exchange nor contributes text; the assistant lines after it attach to the
open exchange. Two deliberate limits keep this conservative:
- isMeta alone does not mean "not the user's input". Channel-bridge plugins
(Discord, Slack, ...) deliver the user's own words as isMeta:true lines with
origin.kind === "channel" (anthropics/claude-code#44828). Provenance is read
from origin.kind, and channel/human lines are treated as normal prompts.
- With no exchange open (a session that opens with a notification), the line
still starts one, exactly as before, so no assistant text is ever dropped.
Codex/Cursor/opencode/OMP parsers are unchanged.
Tests: four fixtures (image paste; skill invocation with pre-tool assistant
text, injected skill body and a "Continue" line; a channel-bridge prompt after
a typed turn; a session opening with a task notification) and six parser tests.
Full suite passes (with TZ=UTC; test/show.test.ts has a pre-existing
host-timezone dependency addressed separately).
Note: exchange ids are position-based, so already-indexed files are not
re-processed by the incremental indexer; their rows must be deleted and
re-indexed to pick up the fix.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7qt58cTm5rFNfV38EQfEN
…context>, ...) no longer splits an exchange Same bug class as the previous commit, in the Codex rollout parser. Codex injects system context as ordinary user-role message items with no field that distinguishes them from typed prompts: the AGENTS.md instructions block, <environment_context>, <recommended_plugins>, and the <skill> body that follows a skill invocation (developer-role wrappers such as <multi_agent_mode> never reach the user branch). parseCodexConversation() called startExchange() on every user item and finalizeExchange() drops an exchange with no assistant text, so when one of these landed between the typed prompt and the reply the prompt was discarded and the injected item became user_message. In 21 local rollouts this happened twice (one <skill>, one <recommended_plugins>); the <skill> case is systematic: every skill invocation loses its prompt. Fix, kept deliberately narrow: - A block counts as injected only when it is a COMPLETE canonical fragment: one input_text block that is exactly one wrapper element (<environment_context>, <recommended_plugins>, <skill>, legacy <user_instructions>, <subagent_notification> — the ones Codex emits with role "user", observed in rollouts or defined in codex-rs) or the "# AGENTS.md instructions[ for <dir>]" + <INSTRUCTIONS> block. "Exactly one" is enforced: the first matching closing tag must end the block, so several elements, or text between them, are user text. Prefix matching is not used: a typed Maven <plugin> snippet or a heading that resembles the AGENTS.md header stays a prompt. - Classification is per content block, so a real request that shares an item with an injected block is kept as the prompt. - While an exchange is open, an injected-only item neither starts a new exchange nor contributes text; the reply attaches to the typed prompt. With no exchange open it still starts one, exactly as before. - Positive provenance wins: newer rollouts log each typed prompt as an item_completed UserMessage event (legacy: user_message). If such an event vouches for the original text of an item that was held back or trimmed, the full authored prompt is restored. - <user_query>, <user_shell_command>, <user_action> and <image ...> + caption carry the user's input and are not listed. Tests: nine cases in test/codex-transcripts.test.ts — the real rollout shape (AGENTS.md, environment_context, typed prompt + UserMessage event, <skill> body, reply → one exchange anchored at the prompt); context re-injected between turns in single and multi-block items; a real request sharing an item with an injected block; look-alike typed prompts (<plugin> XML + question, "# AGENTS.md instructions need cleanup", a pasted <skill> element with trailing text); a UserMessage event overriding the heuristic; several wrapper elements with text between them kept as user text; a mixed item restored in full by its UserMessage event; user-input wrappers and image captions; an injected item opening a session. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7qt58cTm5rFNfV38EQfEN
obra
left a comment
There was a problem hiding this comment.
Reviewed at a70c6b3 against base 28f0933. The core mechanism is sound and the diagnosis is correct — this is a real bug, the scale claim holds up, and the tests are largely well-built. Two blockers below, both about the fix removing content it shouldn't. A second reviewer is still working the diff; I'll add anything further.
I verified the two blocking findings against the source and against real data on this machine rather than relaying them.
Blocker 1 — subagent_notification is not boilerplate; suppressing it destroys real content
src/parser.ts:845 includes subagent_notification in CODEX_INJECTED_OPEN alongside environment_context, recommended_plugins, skill, and user_instructions. Those four are harness plumbing. This one isn't. Sampling a real rollout on this machine, the tag body is a completed subagent's full report:
{"agent_path":"019ee37c-…","status":{"completed":"Jesse, read-only audit done. I did not edit anything.
**Current Docs**
- [docs/job-control.md](…:537)
Replace examples that list `assistant.message` …
That is exactly the kind of text someone later searches for. Measured over 400 real rollouts, one transcript goes 107 → 74 exchanges with indexed user-side text dropping 59,943 → 7,212 chars (−88%): 33 exchanges disappear, each a subagent notification plus a substantive assistant reply. Worse than losing them, the assistant halves get merged backwards into an unrelated prompt, so one embedding ends up covering a short human question plus ~30 replies that have nothing to do with it.
Fix is one token: drop subagent_notification from that regex. It fails the test the other four pass — "does this carry content a human would search for."
Blocker 2 — isMeta is fail-closed, and hook-wrapped user text has no origin
src/parser.ts:179-183:
function isInjectedMetaLine(msg: JSONLMessage): boolean {
if (msg.isMeta !== true) return false;
const kind = msg.origin?.kind;
return !(typeof kind === 'string' && HUMAN_ORIGIN_KINDS.has(kind));
}The origin.kind carve-out is the right idea, but it can only rescue lines that have an origin. Harness-wrapped user text often doesn't, so anything isMeta:true without origin is assumed injected. Confirmed losses in real transcripts, all of them the user's own words:
"wake up and continue if the api fell over"— the user's own loop prompt, redelivered by the harness asisMeta:true, noorigin."Stop hook feedback:\n[ok. we don't want a custom dashboard for evals we haven't run in months…]"— typed steering, wrapped by a hook,isMeta:true, noorigin.
Stop hook feedback appears on 390 isMeta user lines against 3 non-meta in the sampled corpus, so this is a class, not a curiosity. It's also self-demonstrating: the goal text driving my own session today arrives in exactly this shape and would be dropped.
Either exempt the hook/scheduled-prompt shapes, or document the loss explicitly. I'd rather not see it merged silently — the whole point of this PR is that content was disappearing without anyone noticing.
Non-blocking
3. A new test cannot fail, and its name overpromises. test/codex-transcripts.test.ts:378 — "ignores user-role context re-injected between turns (single and multi-block items)" passes with the entire parser change reverted, and survived eight targeted mutations. On base, the injected item starts an exchange that finalizeExchange() drops for having no assistant message, so the observable result is identical either way. The name also claims "single and multi-block" while the body has exactly one item, which is multi-block. Name-asserts-broad / body-checks-narrow is a defect this repo has produced repeatedly; worth fixing while it's cheap.
4. Untested reset. src/parser.ts:1082 — deleting pendingInjected = null; kills no test.
5. Question, not a finding. src/parser.ts:1064: vouched !== undefined && (!vouched.trim() || …) treats an empty vouch as confirmation. If a Codex build emits a content shape extractTextFromContent can't read, every held item is restored and the Codex half silently no-ops. Zero rollouts on this machine use that path (all 7,569 use the legacy user_message event), so it may be unreachable — a defensive vouched.trim() === pending.text.trim() would close it cheaply.
What checked out
distis honest.npm run buildreproduces the committeddist/parser.jsbyte-identical.dist/mcp-server.jsdoesn't bundle parser logic at all, so no second copy ships behind the reviewed source. This was the check I most wanted done, sincedist/is tracked here.- No Codex over-suppression in practice: across 600 real rollouts, 606 items classify as injected and none is followed by a
user_messageevent, while 600 typed items each are. The probe demonstrably fires, so that's a real negative, not a silent one. - Full suite 363/363, no existing behavior changed. Mutation testing kills 6 of 13 new tests on a full revert, and every remaining one except finding 3 dies to a plausible single-line mutation. The four new Claude fixtures do exercise distinct paths.
- Clean merge against current
origin/main(7e06519). - One earlier concern — that the PR opens a new indexer straddle hole — was withdrawn after measurement: across 39 transcripts × 3 cut points, base produces 85 straddling cases and the PR 99. It amplifies a pre-existing limitation ~16% rather than introducing one. Still worth a note that already-indexed exchanges aren't corrected without a re-index, since the 257 figure in your description stays wrong until then.
REQUEST CHANGES, narrowly: blocker 1 is a one-token fix, blocker 2 needs a decision rather than necessarily code. Everything else is cleanup. The underlying work here is good and I want it in.
|
Second reviewer finished. It reached blocker 2 independently and quantified it much more severely than I did, plus found something neither of us had. Escalating. Blocker 2 is worse than I wrote: the Claude half is net-negative as it standsI said "exempt the hook shapes or document the loss." The measurement says that undersells it. Differential over 395 real Claude transcripts, base parser vs PR parser: Examples of what disappears, each
And the shape of the damage, from a fixture: The prompt vanishes and its assistant output is glued onto an unrelated preceding exchange, so the answer is still in the index but attributed to a question nobody asked. Recovering 182 prompts while removing 1,266 is a net loss on the exact axis this PR exists to improve. The Codex half is genuinely good — measured as a no-op or an improvement on real data. It's specifically New finding — the comment claims a coverage it doesn't have
So the large majority carry (The 23 that are Where the two reviewers disagreed, and how I called itThey split on I read the actual bytes before deciding, and the tag body is a completed subagent's full report — audit findings, file paths, recommendations. I'm siding with B: it's tagged like plumbing but carries content people search for. Blocker 1 stands. Flagging the disagreement because "it's a recognized wrapper tag" was a reasonable read, and it's the reason this needs a human call rather than a rule. Confirmed twice, independently
On the re-index question, where they disagreed tooReviewer A simulated a base-era sync point and found 10 files / 125,231 assistant chars that would never be indexed, because merged exchanges now start earlier and fall below an existing high-water mark. Reviewer B measured the related straddle behavior across 39 transcripts × 3 cut points and got 85 cases on base vs 99 on the PR — i.e. a ~16% amplification of something pre-existing. Those measure different things and I haven't reconciled them, so I'm not asserting a number. What's solid: already-indexed data isn't corrected, and the 257-exchange figure in your description stays wrong until a re-index. Worth a line in the PR body either way. Still REQUEST CHANGES. The Codex half is ready. The Claude half needs the suppression narrowed before it ships — as written it removes seven times more real user content than it recovers. |
Problem
When a Claude Code prompt includes a pasted image, the transcript contains two consecutive
type:"user"lines:textblock"[Image #1] <the user's words>"plus animageblock;"isMeta": truewhose only text is"[Image: source: /path/to/file.jpg]"(or"[Image: original WxH, displayed at ...]").parseClaudeConversationstarts a new exchange on every user line andfinalizeExchange()drops any exchange with zero assistant messages. No assistant reply sits between (1) and (2), so the real prompt is discarded and the placeholder becomesuser_message. Text and vector search can't find those conversations by what the user actually asked.The same mechanism mis-handles every other harness-injected
isMetauser line: the Skill tool's injected skill body,[SYSTEM NOTIFICATION - NOT USER INPUT],<local-command-caveat>, subagent coordinator messages, andContinue from where you left off.. A skill invocation, for example, gets split into two exchanges with the skill body indexed as if the user had typed it.Scale on one real index: 257 of 6,448 exchanges (63 transcripts) had an image placeholder as their user message.
Same bug in the Codex parser (second commit)
Codex injects system context as ordinary user-role message items with no distinguishing field: the AGENTS.md instructions block,
<environment_context>,<recommended_plugins>, and the<skill>body after a skill invocation (developer-role wrappers like<multi_agent_mode>never reach the user branch).parseCodexConversation()started an exchange on every user item, so when one of these landed between the typed prompt and the reply, the prompt was dropped. In 21 local rollouts this happened twice; the<skill>case is systematic — every skill invocation loses its prompt.Fix
isMetaandorigintoJSONLMessage.isMetauser line neither starts a new exchange nor contributes text; the assistant lines after it attach to the open exchange (the real prompt).origin.kind, not fromisMetaalone. Channel-bridge plugins (Discord, Slack, …) deliver the user's own words asisMeta:truelines withorigin.kind === "channel"(Rewind picker is unusable in sessions driven by plugin channel notifications (isMeta:truefilter) anthropics/claude-code#44828); those, andorigin.kind === "human", are treated as normal prompts.<environment_context>/<recommended_plugins>/<skill>/ legacy<user_instructions>/<subagent_notification>element (the wrappers Codex emits with roleuser), or the# AGENTS.md instructions[ for <dir>]+<INSTRUCTIONS>block. "Exactly one" is enforced (the first matching closing tag must end the block), and there is no prefix matching, so a typed<plugin>XML snippet, a look-alike heading, or several elements with text between them stay a prompt. Classification is per content block, so a real request sharing an item with an injected block is kept. Same open-exchange rule. Positive provenance wins: anitem_completedUserMessageevent (legacyuser_message) that vouches for the original text of a held-back or trimmed item restores the full authored prompt.<user_query>,<user_shell_command>,<user_action>and<image …>+ caption are not listed.Tests
Four fixtures + six tests in
test/parser.test.ts:image-paste-conversation.jsonl— real prompt +isMetaplaceholder + reply → one exchange,userMessageis the real prompt, anchored at line 1,thinkingLevelfrom the real prompt.skill-invocation-conversation.jsonl— prompt → assistant text → Skilltool_use→tool_result→isMetaskill body → reply →isMeta"Continue…" → reply → one exchange, pre-tool text and both replies kept,toolCalls == ['Skill'].channel-plugin-conversation.jsonl— typed turn, then a Discord-channel prompt (isMeta:true,origin.kind:"channel") → two exchanges; the channel message is a real prompt.meta-first-conversation.jsonl— session opening with a task notification → the notice still anchors an exchange (unchanged from today), the typed prompt after it is separate.Plus nine Codex cases in
test/codex-transcripts.test.ts: real rollout shape with<skill>body andUserMessageevent; context re-injected between turns (single and multi-block); a real request sharing an item with an injected block; look-alike typed prompts (<plugin>XML + question,# AGENTS.md instructions need cleanup, pasted<skill>with trailing text); aUserMessageevent overriding the heuristic; several wrapper elements with text between them; a mixed item restored in full by its event; user-input wrappers and image captions; an injected item opening a session.Full suite passes with
TZ=UTC.test/show.test.tshas a pre-existing host-timezone/locale dependency, fixed in a separate PR.Upgrade note
Exchange ids are
md5(archivePath:userLine-lastAssistantLine), so affected exchanges get new ids after the fix. Because the indexer resumes fromMAX(line_end)per file, already-indexed transcripts are not re-processed automatically; stale placeholder rows persist until those files are re-indexed (delete their rows, then re-run indexing). Count them with:🤖 Generated with Claude Code
https://claude.ai/code/session_01L7qt58cTm5rFNfV38EQfEN