6krrt/code_plans/shared-helper-unparseable-input-contract.md

Spec + implementation plan: pin down what a shared helper yields on well-typed-but-unparseable input

Origin. PR #3 collapsed four independent messages → image_url part traversals into one generator, capabilities.iter_image_url_values (code_reviews/capability-gate-followups.md, item #6). The plan specified the two shapes the four originals agreed on (image_url as a dict with url, or as a bare string) but not the case where a part is typed image_url and carries neither — well-typed, but nothing to extract. The merged helper silently skipped that case instead of yielding a placeholder, which dropped a degenerate part out of two fail-closed checks at once (see code_reviews/pr-3-capability-gate-followups-review.md for the live repro and the fix). The follow-up opencode session that implemented the fix named the general lesson explicitly, which is what this document is formalizing: a plan that proposes collapsing N implementations into one shared helper has to specify the helper's contract on every input shape, not only the ones the N originals happened to agree on.

This is a process document, not a bug report — no code changes accompany it. It exists so the next plan that proposes a shared helper pins this down up front instead of it being rediscovered in review.

---

Spec

The problem, generalized

When several independent implementations of "walk this structure and pull out X" get unified into one shared helper, each original's handling of its own edge cases has to be re-derived by the person writing the unification — nothing forces them to enumerate it, because the edge case was never the reason any of the N implementations were written in the first place. Each one just happened to fall into some fallback path. The happy path is easy to unify because all N agree on it by construction (they all worked, on real traffic, for the common shape). The edge cases are exactly where they are most likely to quietly disagree, and disagreement there is invisible unless a plan makes each implementation's edge-case behavior explicit before proposing the replacement.

The specific edge case that bit PR #3 — matches the coarse type tag but the payload is missing or empty — deserves its own name because it is a distinct category from "wrong type" or "absent": it is a well-typed but unparseable input. A traversal that filters on part.get("type") == "image_url" before doing anything else will always let this shape through the filter; what happens next is the part that has to be decided deliberately.

The rule

Any implementation plan that proposes extracting or unifying a shared traversal, parser, or extractor helper from multiple existing call sites must include an explicit contract: for every input shape the helper will see, state what it yields or returns. At minimum, enumerate:

1. The common/valid shape — the one all N originals handle the same way.
2. Every alternate valid shape any existing call site already handles (there may be more than one — PR #3 had three: image_url as {"url": ...}, image_url as a bare string, and a bare url key on the part itself).
3. The well-typed-but-unparseable shape — passes the coarse type/shape check but the value cannot actually be extracted. State the yielded value explicitly (a sentinel like "", not "nothing" / "skip") unless skipping is a deliberate, stated decision.
4. The wrong-typed or absent shape — fails the coarse check. Usually "skip", but say so, so it's a decision and not an assumption.

And critically: for category 3, list what each of the N originals currently does, not just what the new helper will do. That list is what surfaces disagreement before the merge, rather than after. In PR #3, all four originals agreed by accident (each one's "nothing extracted" path happened to either count the part or fail closed) — but nothing in the plan recorded that agreement, so the merge didn't have to preserve it and didn't.

Why "skip silently" is the default failure mode to watch for

A for/if/continue traversal skips by construction unless a branch explicitly appends/yields on every path. Converting four such loops into one yield-per-match generator is a natural, idiomatic simplification — and it's exactly the transformation that turns "always contributes something, even a fail-closed placeholder" into "contributes nothing, silently, for a whole category of input." This isn't specific to generators — the same risk applies to a shared parser that returns None for the unparseable case where callers previously each had their own None-handling that didn't all agree — but generators make it easy to introduce, because continue-without-yielding reads as the most natural thing to write.

---

Implementation plan

This is a discipline to apply going forward, not a code change. Concretely:

1. Template for future plans

When a code_plans/ (or code_reviews/) entry proposes extracting a shared helper from multiple call sites, include a table shaped like this alongside the proposed code:

Input shape	Original A	Original B	Original C	...	New shared helper
common/valid	...	...	...		...
alternate valid #1	...	...	...		...
well-typed, unparseable	...	...	...		... (state the sentinel)
wrong-typed / absent	skip	skip	skip		skip

If any two originals disagree in a row, that disagreement is the thing the plan has to resolve on purpose (pick one, document why) rather than let the merge resolve by accident.

2. No retroactive audit needed right now, but a watch list for next time

Checked the rest of dispatcher.py for other traversals of a message's multimodal content list, since that's the same shape that bit PR #3. None of these are currently merged/unified helpers, so none are broken — they're independent, single-purpose traversals, not N-collapsed-into-1. But if any of them are ever unified (with each other, or with iter_image_url_values), this spec applies:

• session_fingerprint (dispatcher.py:1302) — joins text parts to fingerprint a session.
• session_directory (dispatcher.py:1321) — joins text parts to find a working directory.
• estimate_prompt_tokens (dispatcher.py:1364) — sums text-part lengths for the context estimate.
• _last_user_text (dispatcher.py:1391) — joins the last user turn's text parts.

All four already treat a non-dict or non-string part as "contributes nothing" consistently with each other (each filters with isinstance(p, dict) / isinstance(p.get("text"), str) before including it), so there's no live disagreement to flag today — this is a watch list, not a finding.

3. Where this lives

Keep this document as the reference the next relevant plan cites, rather than duplicating the rule inline each time. A future plan proposing a shared-helper extraction should link back to this file (code_plans/shared-helper-unparseable-input-contract.md) and fill in the contract table from §1 as part of the plan, the same way code_reviews/capability-gate-followups.md gave concrete before/after code for each finding rather than describing it in prose only.