claude-mem/PATHFINDER-2026-04-21/07-plans/03-response-parsing-storage.md

03 — response-parsing-storage (implementation plan)

Design authority: 05-clean-flowcharts.md §3.7 (clean diagram + deletion list at lines 295–317), Part 1 bullshit items #20–#23 (lines 38–41), Part 2 decision D5 (lines 77). This plan translates §3.7 into concrete edits. Where the audit disagrees with verified code, the live-file citations win and are called out.

Dependencies

• Upstream — 02-sqlite-persistence. The sibling plan introduces a UNIQUE(session_id, tool_use_id) constraint on pending_messages and replaces the 30 s in-memory dedup window with INSERT … ON CONFLICT DO NOTHING. This plan does not touch pending_messages schema, but the sibling's markFailed contract (UPDATE … SET status='failed') must remain intact — parser-level failure marking continues to go through PendingMessageStore.markFailed(messageId) at src/services/sqlite/PendingMessageStore.ts:349. Cite: 02-sqlite-persistence Phase 2 (UNIQUE-constraint phase).
• Downstream — 07-session-lifecycle-management. That plan owns RestartGuard evolution and the one-reaper timer. Critical coupling: today RestartGuard (src/services/worker/RestartGuard.ts:12–70) exposes only recordRestart(), recordSuccess(), and read-only counters — there is no recordFailure() method. The audit's D5 claim "RestartGuard already exists for repeated failures" is half-true: it covers process-restart loops, not per-message parse failures. Two legitimate options:
  1. (preferred) Let parse-failure propagate via PendingMessageStore.markFailed only. Session exits through the existing idle path; on the next summarize or observation attempt the session is re-initialised. If parsing fails repeatedly enough to crash the SDK subprocess, RestartGuard.recordRestart() is the thing that trips — already wired via existing restart paths. No new RestartGuard surface area required.
  2. (alt) Add session.recordFailure(reason) as a thin helper that logs + calls markFailed for each processingMessageIds entry. Still no RestartGuard API changes. This plan adopts option (1): no new methods on RestartGuard. The flowchart box "session.recordFailure()" from §3.7 resolves to the block of code that marks all processingMessageIds as 'failed' in pending_messages — identical shape to today's non-XML early-fail branch at ResponseProcessor.ts:102–106, but reached through the single parseAgentXml return path. See the 07-session-lifecycle-management plan for any RestartGuard API additions; do not add them here.

Verified facts (pinned to files)

#	Fact	Source
V7a	coerceObservationToSummary is a private fn used twice inside parseSummary.	src/sdk/parser.ts:222 (def), :152 + :197 (call sites)
V7b	Non-XML early-fail branch lives at lines 87–108.	src/services/worker/agents/ResponseProcessor.ts:87–108
V7c	Consecutive-summary-failures circuit breaker lives at lines 176–200.	src/services/worker/agents/ResponseProcessor.ts:176–200
V7d	consecutiveSummaryFailures field on ActiveSession.	src/services/worker-types.ts:53
V7e	consecutiveSummaryFailures is also read by SessionManager.queueSummarize at line 340 to short-circuit. That site must be deleted too — the original Phase 3 draft in 06-implementation-plan.md did not list it.	src/services/worker/SessionManager.ts:340–346
V7f	MAX_CONSECUTIVE_SUMMARY_FAILURES constant in src/sdk/prompts.ts:21 is imported by both ResponseProcessor.ts:16 and SessionManager.ts (via prompts import). Delete the constant and both imports.	src/sdk/prompts.ts:21
V7g	Pending-message FAILED state literal is 'failed' (lowercase). CHECK constraint: status IN ('pending','processing','processed','failed'). markFailed(messageId) is the official API.	src/services/sqlite/PendingMessageStore.ts:22, :349, :369; src/services/sqlite/migrations/runner.ts:533; src/services/sqlite/SessionStore.ts:565
V7h	RestartGuard has no recordFailure() method. Public surface: recordRestart(), recordSuccess(), restartsInWindow, windowMs, maxRestarts.	src/services/worker/RestartGuard.ts:1–70
V7i	Prompts already mandate <summary> root tag for summary turns ("you MUST wrap your ENTIRE response in <summary>...</summary> tags", "The ONLY accepted root tag is <summary>"). <skip_summary reason="..."/> is recognised by the parser (parser.ts:124) but is not documented in buildSummaryPrompt as a valid alternative. Prompt must be updated (Phase 1b) so the D5 contract is actually printed to the agent.	src/sdk/prompts.ts:153–174; src/sdk/parser.ts:124
V7j	Atomic TX boundary is sessionStore.storeObservations(...) (single call, internal BEGIN/COMMIT). Do not split it. Today it wraps observations + optional summary in one transaction.	src/services/worker/agents/ResponseProcessor.ts:149–164, src/services/sqlite/observations/store.ts (module)
V7k	parseSummary accepts coerceFromObservation: boolean = false. All coercion is gated on this flag — it is true only when summaryExpected (derived from SUMMARY_MODE_MARKER substring match) is true.	src/sdk/parser.ts:122, ResponseProcessor.ts:75–81

Concrete target signatures

// src/sdk/parser.ts — replaces parseObservations + parseSummary + coerceObservationToSummary
export type ParseFailureReason = 'no_xml' | 'missing_summary' | 'malformed';

export interface ParsedAgentOutput {
  observations: ParsedObservation[];
  summary: ParsedSummary | null;
  skipSummary: boolean;
}

export type ParseResult =
  | { valid: true; data: ParsedAgentOutput }
  | { valid: false; reason: ParseFailureReason };

export function parseAgentXml(
  text: string,
  opts: { requireSummary: boolean; correlationId?: string; sessionId?: number }
): ParseResult;

Failure semantics (no coercion, per D5):

• text.trim() is non-empty, no <observation>/<summary>/<skip_summary token → {valid:false, reason:'no_xml'}.
• opts.requireSummary === true and parse yields no <summary> and no <skip_summary/> → {valid:false, reason:'missing_summary'}.
• Any regex match with empty sub-tag payload where requireSummary → {valid:false, reason:'malformed'}.
• Otherwise → {valid:true, data:{observations, summary|null, skipSummary}}.

Phases

Phase 1 — Write parseAgentXml in src/sdk/parser.ts

(a) What to implement

1. Copy extractField from src/sdk/parser.ts:267–276 and extractArrayElements from :282–305 verbatim into the new module layout. These remain private helpers.
2. Copy the observation-extraction loop body (field extraction, type validation, ghost-obs filter) from src/sdk/parser.ts:40–108 into a private extractObservations(text, correlationId) that returns ParsedObservation[]. No behaviour change.
3. Copy the summary-extraction happy path (skip_summary check at :124–133, <summary> regex at :136–137, field extraction at :164–169, false-positive guard at :191–214) into a private extractSummary(text, sessionId) that returns { summary: ParsedSummary | null; skipSummary: boolean; malformed: boolean }. Delete the two coerceFromObservation branches at :151–158 and :196–203 — they do not survive.
4. Delete coerceObservationToSummary (src/sdk/parser.ts:222–259, 38 lines) outright.
5. Write the public parseAgentXml(text, opts) that:
   • Computes observations = extractObservations(text, opts.correlationId).
   • Computes { summary, skipSummary, malformed } = extractSummary(text, opts.sessionId).
   • Returns {valid:false, reason:'no_xml'} if text.trim() && observations.length === 0 && !summary && !skipSummary && !/<observation>|<summary>|<skip_summary\b/.test(text).
   • Returns {valid:false, reason:'missing_summary'} if opts.requireSummary && !summary && !skipSummary.
   • Returns {valid:false, reason:'malformed'} if opts.requireSummary && malformed.
   • Returns {valid:true, data:{observations, summary, skipSummary}} otherwise.
6. Remove the old named exports parseObservations and parseSummary and their coerceFromObservation parameter. Keep ParsedObservation/ParsedSummary interfaces (src/sdk/parser.ts:9–27) — they're part of the public shape.

(b) Docs — 05-clean-flowcharts.md §3.7 (clean diagram, lines 295–317), Part 1 #20/#21/#23 (lines 38–41), Part 2 D5 (line 77). V7a (parser.ts:222). V7i (prompt contract already mandates <summary>; skip-summary token recognised at parser.ts:124). V7k (coerceFromObservation gating on summaryExpected).

(c) Verification

• grep -n "coerceObservationToSummary" src/ → 0 hits.
• grep -n "parseObservations\|parseSummary\b" src/ → 0 hits outside parser.ts itself; inside parser.ts only the private helpers.
• Unit test: parseAgentXml('', {requireSummary:false}) → {valid:true, data:{observations:[], summary:null, skipSummary:false}} (empty string is not no_xml; trim is empty).
• Unit test: parseAgentXml('Error: auth token expired', {requireSummary:true}) → {valid:false, reason:'no_xml'}.
• Unit test: agent returns <observation><type>x</type><title>t</title></observation> with requireSummary:true → {valid:false, reason:'missing_summary'} (no coercion to summary).
• Unit test: <skip_summary reason="no work"/> with requireSummary:true → {valid:true, data:{observations:[], summary:null, skipSummary:true}}.
• Unit test: <summary><request>r</request>…</summary> → {valid:true, data:{…, summary:{…}, skipSummary:false}}.

(d) Anti-pattern guards

• Guard C (silent fallback): Coercion is deleted, not relocated. grep -n "coerce" src/sdk/parser.ts → 0 hits.
• Guard D (facades): parseObservations + parseSummary collapse to a single parseAgentXml. Two public fns → one.
• Guard A (invent APIs): No new classes. Pure function returning a discriminated union. No ParserValidator, no SummaryCoercer, no base class.

---

Phase 1b — Update agent contract in src/sdk/prompts.ts

(a) What to implement — Extend buildSummaryPrompt() at src/sdk/prompts.ts:140–175 (the return-value template) so it explicitly permits <skip_summary reason="..."/> as an alternative when there is literally nothing to summarise. Current text says "The ONLY accepted root tag is <summary>" (:155), which is incompatible with the parser's <skip_summary/> recognition (parser.ts:124) and incompatible with the D5 contract ("<summary> or <skip_summary/>"). Proposed insertion, directly after the existing line :173:

• If (and ONLY if) there is no work to summarise, you may return
  <skip_summary reason="..."/> as the sole root tag instead of <summary>.
  Any other response is a protocol violation and the session will fail.

Also delete the export MAX_CONSECUTIVE_SUMMARY_FAILURES at src/sdk/prompts.ts:21 and its JSDoc at :17–20. The constant is unused after Phase 2 + Phase 3.

(b) Docs — §3.7 deletion list ("agent must return <summary> or <skip_summary/>", line 311). Part 2 D5 (line 77). V7i.

(c) Verification

• grep -n "MAX_CONSECUTIVE_SUMMARY_FAILURES" src/ → 0 hits.
• Manual diff of generated summary prompt shows the skip-summary clause.
• Existing prompt-mandate text (:153, :155, :173) preserved so the normal-case contract stays strict.

(d) Anti-pattern guards

• Guard C: The contract is now self-describing — no silent downstream coercion needed because the agent is told the protocol explicitly.

---

Phase 2 — Replace parse path in ResponseProcessor.ts

(a) What to implement

1. Replace the import at src/services/worker/agents/ResponseProcessor.ts:15 with import { parseAgentXml, type ParsedObservation, type ParsedSummary } from '../../../sdk/parser.js';. Delete MAX_CONSECUTIVE_SUMMARY_FAILURES from the :16 import (keep SUMMARY_MODE_MARKER).
2. Replace processAgentResponse body at :69–108:
   • Keep :62–67 (lastGeneratorActivity + conversationHistory append).
   • Compute summaryExpected exactly as today (:75–79).
   • Replace :70 and :81 (two separate parse calls) with a single call:

     const parsed = parseAgentXml(text, {
       requireSummary: summaryExpected,
       correlationId: session.contentSessionId,
       sessionId: session.sessionDbId,
     });
     

   • Replace the non-XML early-fail block :83–108 (26 lines) with:

     if (!parsed.valid) {
       const preview = text.length > 200 ? `${text.slice(0, 200)}...` : text;
       logger.warn('PARSER', `${agentName} returned invalid response (${parsed.reason}); marking messages as failed`, {
         sessionId: session.sessionDbId,
         reason: parsed.reason,
         preview,
       });
       const pendingStore = sessionManager.getPendingMessageStore();
       for (const messageId of session.processingMessageIds) {
         pendingStore.markFailed(messageId);
       }
       session.processingMessageIds = [];
       return;
     }
     const { observations, summary } = parsed.data;
     

   • Everything at :110–174 stays unchanged (normalize, ensureMemorySessionIdRegistered, STORING log, labeledObservations, atomic TX, STORED log, lastSummaryStored) — the single-TX invariant is preserved.
3. Delete the circuit-breaker block :176–200 (25 lines) entirely. After deleting, :202 (claim-confirm) runs immediately after :174 (lastSummaryStored).
4. No changes to :202–241 (claim-confirm, restartGuard.recordSuccess, Chroma sync, SSE broadcast, cleanup).
5. (Preflight edit 2026-04-22 — reconciliation C6) Emit summaryStoredEvent when a summary row is committed. After setting session.lastSummaryStored (unchanged from today), if session.summaryStoredEvent exists (initialized by SessionManager when the session is created, see plan 07 Phase 7), call session.summaryStoredEvent.emit('stored', summaryId). This unblocks the blocking /api/session/end handler in plan 07 Phase 7 without polling. Contract: emit exactly once per summary commit; summaryId is the newly inserted row id from the atomic TX.

   // inside the block that sets session.lastSummaryStored (around :170–174)
   session.lastSummaryStored = true;
   session.summaryStoredEvent?.emit('stored', summaryRowId);
   

(b) Docs — §3.7 clean diagram (B→C→D→{Fail | Store}→Confirm→…, lines 299–308). Part 1 #21 (line 39), #22 (line 40). Part 2 D5 (line 77). V7b (:87–108), V7c (:176–200), V7g ('failed' + markFailed).

(c) Verification

• grep -n "parseObservations\|parseSummary\|coerceObservationToSummary\|consecutiveSummaryFailures" src/services/worker/agents/ResponseProcessor.ts → 0 hits.
• grep -n "MAX_CONSECUTIVE_SUMMARY_FAILURES" src/services/worker/agents/ResponseProcessor.ts → 0 hits.
• Integration test A — malformed input: send "Service temporarily unavailable" as text, assert (i) no row inserted in observations table, (ii) no row in session_summaries, (iii) every id in session.processingMessageIds has status='failed' in pending_messages after the call returns, (iv) session.processingMessageIds === [].
• Integration test B — observation-without-summary when summary expected: summaryExpected=true, text is <observation><type>code</type><title>x</title></observation>, assert (i) no row in session_summaries, (ii) no row in observations (contract failure fails the whole batch — no partial write), (iii) pending messages marked failed. This is the critical regression test — today the coerce path would have written a coerced summary row.
• Integration test C — valid obs + summary: single atomic TX still commits both rows together (pre-existing behaviour, no regression).

(d) Anti-pattern guards

• Guard C: No coercion, no "close-enough" branch. Every parsed.valid === false path leads to markFailed and return.
• Guard D: One parse call (parseAgentXml) replaces two (parseObservations + parseSummary). No wrapper facade.
• Guard A: No new method on RestartGuard, no new class, no new helper file. Direct calls to the existing PendingMessageStore.markFailed.

---

Phase 3 — Remove consecutiveSummaryFailures from ActiveSession + its consumer

(a) What to implement

1. Delete src/services/worker-types.ts:51–53 (the three lines: JSDoc + consecutiveSummaryFailures: number; field). Field name must vanish from the type.
2. Delete src/services/worker/SessionManager.ts:336–346 (the 11-line circuit-breaker check in queueSummarize). The method body goes straight from the auto-initialize check (:331–334) to the // CRITICAL: Persist to database FIRST comment (:348). This deletion was omitted from the original Phase 3 draft at 06-implementation-plan.md:155–204 — V7e is the new citation.
3. Delete the initialiser consecutiveSummaryFailures: 0, at SessionManager.ts:232 (inside initializeSession).
4. Delete the MAX_CONSECUTIVE_SUMMARY_FAILURES import in SessionManager.ts (if present). Use grep -n "MAX_CONSECUTIVE_SUMMARY_FAILURES" src/services/worker/SessionManager.ts first; remove the line.
5. No schema changes. No new RestartGuard API (see Dependencies above — option (1)).

(b) Docs — §3.7 deletion bullet "consecutiveSummaryFailures counter + circuit-breaker logic (RestartGuard covers this already)" (line 314). Part 1 #22 (line 40). Part 2 D5 (line 77). V7d, V7e, V7f.

(c) Verification

• grep -rn "consecutiveSummaryFailures" src/ → 0 hits.
• grep -rn "MAX_CONSECUTIVE_SUMMARY_FAILURES" src/ → 0 hits (constant, its JSDoc, all imports gone).
• TypeScript compile succeeds (removing a field and all references is mechanical; no union fallout expected).
• Behavioural test: call sessionManager.queueSummarize(sessionDbId) five times in rapid succession with intentionally failing agent output; assert every call enqueues to pending_messages (no silent drop) and each failed attempt marks that message 'failed'. The old circuit breaker would have swallowed calls 4–5; the new contract doesn't.
• Behavioural test: existing RestartGuard still trips after the configured restart count (MAX_WINDOWED_RESTARTS = 10, RESTART_WINDOW_MS = 60_000) — prove that repeated parse failures + subsequent subprocess restarts still converge to guard-tripped within the window. Covered by 07-session-lifecycle-management tests; no duplication here.

(d) Anti-pattern guards

• Guard A: No new RestartGuard.recordFailure() invented. The class stays at 70 lines, public API unchanged. Dependency coupling to 07-session-lifecycle-management is documentation-only.
• Guard C: Removing the circuit breaker means failures flow to queue-level 'failed' state — a single, visible, DB-backed failure signal. No silent swallow.

---

Phase 4 — Verification sweep

(a) What to implement — Grep audit + targeted regression tests. No new code.

(b) Docs — §3.7 full deletion list (lines 310–315), Phase 3 verification block in 06-implementation-plan.md:189–195.

(c) Verification — must all return 0 matches

• grep -rn "coerceObservationToSummary" src/ → 0.
• grep -rn "consecutiveSummaryFailures" src/ → 0.
• grep -rn "MAX_CONSECUTIVE_SUMMARY_FAILURES" src/ → 0.
• grep -rn "parseObservations\|parseSummary" src/ | grep -v "src/sdk/parser.ts" → 0 (the only survivors are private helpers inside parser.ts itself; if you named them without the parse prefix this grep is also 0).
• grep -rn "coerceFromObservation" src/ → 0.

(c-cont) Regression tests — must all pass

• Parser fuzz: feed 1 000 synthetic agent outputs mixing valid/invalid XML + present/absent <summary>; assert valid:false paths never write to observations or session_summaries. Must be 0 coerced summary rows.
• Atomic-TX sanity: inject a DB error on INSERT INTO session_summaries; assert storeObservations rolls back so observations for that batch also revert. (Pre-existing invariant; we didn't touch it, but prove it.)
• Idempotency of failure: double-delivery of the same malformed response (e.g., via worker crash + retry) results in the same pending_messages row in 'failed' status; second attempt does not create a duplicate observation. Relies on upstream 02-sqlite-persistence UNIQUE(session_id, tool_use_id) — cross-check with that plan.
• End-to-end: Stop-hook summarize path exercises parseAgentXml({requireSummary:true}). With a mocked agent returning garbage, assert the hook receives the 110 s timeout path (no silent summary write), the pending message is 'failed', and SessionManager does NOT short-circuit subsequent summarize enqueues (circuit breaker is gone).

(d) Anti-pattern guards — All four grep checks enforce Guards A/C/D structurally.

---

Blast radius

Files modified:

• src/sdk/parser.ts — full rewrite of public surface; private helpers preserved.
• src/sdk/prompts.ts — two-edit surgical change (skip-summary clause, constant delete).
• src/services/worker/agents/ResponseProcessor.ts — replace lines 15–16 imports, 69–108 parse block, delete 176–200 circuit breaker.
• src/services/worker-types.ts — delete 3 lines.
• src/services/worker/SessionManager.ts — delete 11 lines (queueSummarize guard) + 1 line initialiser + maybe 1 import.

Files not touched: src/services/sqlite/observations/store.ts (atomic TX lives here and is preserved). src/services/worker/RestartGuard.ts (API unchanged — see Dependencies option 1). src/services/worker/agents/SessionCleanupHelper.ts. ObservationBroadcaster.ts. Any Chroma sync module.

Schema changes: none.

Estimated lines deleted:

• coerceObservationToSummary body + JSDoc: ~43 lines
• coerceFromObservation branches in parseSummary: ~16 lines
• parseSummary / parseObservations wrapper deduplication: ~15 lines (after collapse into parseAgentXml)
• Non-XML early-fail block in ResponseProcessor.ts:83–108: ~26 lines (replaced by ~12 lines → net –14)
• Circuit breaker in ResponseProcessor.ts:176–200: ~25 lines
• consecutiveSummaryFailures field + initialiser + SessionManager guard: ~15 lines
• MAX_CONSECUTIVE_SUMMARY_FAILURES constant + JSDoc + imports: ~8 lines

Net: ~135 lines deleted, ~35 lines added → ~100 LoC net reduction.

Confidence + gaps

High confidence:

• Parser rewrite is mechanical (extract three private fns, compose them, add the discriminated-union return).
• 'failed' status string + markFailed API are verified.
• Circuit-breaker + field removals are pure deletion once call sites are enumerated (V7e catches the missed site).

Gaps:

1. RestartGuard contract claim in D5 is overstated. D5 says "RestartGuard already exists for repeated failures — delete the separate counter". RestartGuard today only handles process-restart loops, not per-message parse failures. This plan adopts the narrower interpretation (parse failure → markFailed; existing RestartGuard handles the subprocess-restart side effects unchanged). If the 07-session-lifecycle-management plan decides to add RestartGuard.recordFailure(), callers here can start using it in a follow-up — no churn to this plan. Flag for 07-session-lifecycle-management author: confirm the RestartGuard surface they want.
2. Prompt updates assumed in-scope. The audit implies the agent contract "already states <summary> or <skip_summary/>". Verified: prompts enforce <summary> strictly but never mention <skip_summary/>. Phase 1b adds the missing clause. If the team prefers to keep <skip_summary/> as a recognised-but-undocumented escape hatch, Phase 1b can be dropped — but then the parser should be stricter too (reason missing_summary when only skip-summary is emitted without prompt permission). Flag for product owner.