Skip to content

expand: distinguish an unresolvable marker id from a missing stash #111

Description

@amiddavid

expand.RepairToolResults substitutes Unavailable(hashID) for any marker id it cannot
resolve. That is the right behaviour for the model — it reads as "the content aged out" rather
than inviting a retry of a tool that "does not exist" — but the two reasons an id fails to
resolve are not the same event and are currently indistinguishable:

  • Malformed id — the model invented or corrupted a marker id. Needs no action; it is the
    model being the model.
  • Well-formed id with nothing stashed behind it — a context-guru defect. A cut was
    advertised as reversible and it was not. The stash expired, the store did not persist, or the
    key was written under a session id nothing reads.

Both currently land on the same placeholder with no counter, so the second one is invisible. The
process-wide signals that do exist measure something else: RecordExpand / wasted_tokens
counts tokens successfully re-served, and sse_expand_after_stream counts a streaming miss.
Neither can go non-zero for a broken stash — a silent no-op looks identical to "no expand calls
happened".

This is the failure mode that ran unnoticed for three iterations on #80: the refusals were found
by grepping the benchmark client's transcripts, not from any counter in this repo.

Why it was not fixed in #110

#110 is scoped to defects with selection_mode unset and every change in it is
revert-verified. This one is not a behaviour defect but an observability gap, and closing it
needs a design decision that #110 had no basis to make: what counts as a well-formed marker
id.
Marker(key) wraps a store key, so classification depends on the key's shape
(ParseMarkers currently accepts anything between the delimiters). Options:

  1. Validate against the hash format hashKey produces (length + alphabet). Cheap, but couples
    the classifier to the hashing scheme.
  2. Treat "well-formed" as "this proxy could have minted it" — e.g. a short HMAC/prefix over the
    key — which makes the distinction exact but changes marker bytes, and marker bytes are
    prefix-cache-relevant (expand/inject.go).
  3. Classify by store outcome instead of id shape: distinguish "key absent" from "key present but
    empty/expired", if the store can report that. No marker change, but it only separates the
    causes the store can see.

Option 3 looks closest to free if store.Store can distinguish absent from expired; otherwise
option 1 with the coupling documented.

Acceptance

  • Two counters, split by cause, surfaced in /stats and added to the reviewed golden contract
    in proxy/stats_golden_test.go rather than loosening the assertion.
  • A test per cause, each verified to fail when its subject is reverted.
  • The missing-stash counter must be the one an operator can alert on, since it is the one that
    means this proxy broke a promise it made.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions