Skip to content

fix(parser): harness-injected user lines must not split an exchange (Claude isMeta, Codex wrappers) - #168

Open
vicnaum wants to merge 2 commits into
obra:mainfrom
vicnaum:fix/skip-ismeta-user-lines
Open

vicnaum wants to merge 2 commits into
obra:mainfrom
vicnaum:fix/skip-ismeta-user-lines

Conversation

@vicnaum

@vicnaum vicnaum commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Problem

When a Claude Code prompt includes a pasted image, the transcript contains two consecutive type:"user" lines:

  1. the real prompt — a text block "[Image #1] <the user's words>" plus an image block;
  2. a follow-up line flagged "isMeta": true whose only text is "[Image: source: /path/to/file.jpg]" (or "[Image: original WxH, displayed at ...]").

parseClaudeConversation starts a new exchange on every user line and finalizeExchange() drops any exchange with zero assistant messages. No assistant reply sits between (1) and (2), so the real prompt is discarded and the placeholder becomes user_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 isMeta user line: the Skill tool's injected skill body, [SYSTEM NOTIFICATION - NOT USER INPUT], <local-command-caveat>, subagent coordinator messages, and Continue 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

  • Add isMeta and origin to JSONLMessage.
  • 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 (the real prompt).
  • Provenance is read from origin.kind, not from isMeta alone. Channel-bridge plugins (Discord, Slack, …) deliver the user's own words as isMeta:true lines with origin.kind === "channel" (Rewind picker is unusable in sessions driven by plugin channel notifications (isMeta:true filter) anthropics/claude-code#44828); those, and origin.kind === "human", 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: a block is injected only when it is a complete canonical fragment — exactly one <environment_context> / <recommended_plugins> / <skill> / legacy <user_instructions> / <subagent_notification> element (the wrappers Codex emits with role user), 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: an item_completed UserMessage event (legacy user_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.
  • No behaviour change for Cursor, opencode or OMP transcripts.

Tests

Four fixtures + six tests in test/parser.test.ts:

  • image-paste-conversation.jsonl — real prompt + isMeta placeholder + reply → one exchange, userMessage is the real prompt, anchored at line 1, thinkingLevel from the real prompt.
  • skill-invocation-conversation.jsonl — prompt → assistant text → Skill tool_use → tool_result → isMeta skill 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 and UserMessage event; 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); a UserMessage event 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.ts has 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 from MAX(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:

SELECT count(*) FROM exchanges WHERE user_message LIKE '[Image: source:%' OR user_message LIKE '[Image: original%';

🤖 Generated with Claude Code

https://claude.ai/code/session_01L7qt58cTm5rFNfV38EQfEN

vicnaum and others added 2 commits September 10, 2026 16:32
…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 obra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 as isMeta:true, no origin.
  • "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, no origin.

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

  • dist is honest. npm run build reproduces the committed dist/parser.js byte-identical. dist/mcp-server.js doesn't bundle parser logic at all, so no second copy ships behind the reviewed source. This was the check I most wanted done, since dist/ 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_message event, 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.

@obra

obra commented Sep 10, 2026

Copy link
Copy Markdown
Owner

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 stands

I 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:

exchanges:        7,357  ->  6,276
real prompts recovered:      182
messages removed:          1,266   (across 83 of 395 transcripts)
  of which substantive user-authored instructions:  361

Examples of what disappears, each isMeta:true with no origin field — the user's own typed words, replayed by /loop, cron, or an autonomous-loop harness:

  • "Check PR 959 for a new roborev review (run from …)"
  • "Heartbeat tick (recursion campaign, Phase 3 FINALE): …" (1,367 chars)
  • an 11,648-char design brief
  • "Continue the kata execution batch — close the two remaining katas …"

And the shape of the damage, from a fixture:

BASE[0] U="start the release"                A="release started"
BASE[1] U="Check PR 959 … merge if clean."   A="PR 959 reviewed and merged."

PR  [0] U="start the release"   A="release started\n\nPR 959 reviewed and merged."

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 isInjectedMetaLine's default-suppress posture that needs narrowing: key off recognized boilerplate shapes rather than treating "no origin" as proof of injection.

New finding — the comment claims a coverage it doesn't have

src/parser.ts:255-258 says the isMeta path covers "task/system notifications". I measured this myself over 120 local transcripts:

user lines containing <task-notification>:  161
  isMeta === true:                           23
  origin.kind = "task-notification":        152

So the large majority carry origin.kind: "task-notification" and no isMeta — isInjectedMetaLine() requires isMeta === true, returns false, and those lines keep splitting exchanges and keep becoming the indexed user_message. The reviewer reported this as 3,030/3,030 with isMeta absent; my sample says 23 of 161 do carry it, so the absolute form of that claim is too strong. The substance holds either way: the most common notification shape isn't covered, and the comment says it is.

(The 23 that are isMeta:true do get suppressed, since task-notification isn't in HUMAN_ORIGIN_KINDS. So the handling is inconsistent between two populations of the same thing.)

Where the two reviewers disagreed, and how I called it

They split on subagent_notification. Reviewer B measured an 88% content loss and said drop it. Reviewer A classified the same removals as correct — "every removed string was a genuine <subagent_notification>/<environment_context>/<skill> block."

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

  • dist is honest — both reviewers rebuilt from source and got dist/parser.js byte-identical to the committed file. dist/mcp-server.js doesn't bundle parser logic.
  • test/codex-transcripts.test.ts:378 is vacuous — passes with the parser change fully reverted, in both reviews.
  • No assistant text is lost overall (15,370,881 → 15,373,267 chars, the delta being join separators), and no index bloat (max assistant message 44,458 → 50,735, nothing over 200k).
  • Codex user text that merely looks injected is safe: a prefix/suffix wrapper, two wrapper elements in one message, and a user literally typing [Image: original 100x100…] are all preserved. endsAtFirstClose holds.

On the re-index question, where they disagreed too

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants