Skip to content

refactor(runtime): make dispatched attempts own request observations - #4300

Open
Astro-Han wants to merge 13 commits into
mainfrom
refactor/4299-request-observation-authority
Open

refactor(runtime): make dispatched attempts own request observations#4300
Astro-Han wants to merge 13 commits into
mainfrom
refactor/4299-request-observation-authority

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make the canonical dispatched ModelCallAttempt own one bounded, secret-free PreparedRequestObservation
  • derive private serialization, request digest, and ordered semantic segments from one prepared-request representation, including exact binary encoding and explicit opaque redaction
  • keep private artifact persistence best-effort and outside dispatch/accounting liveness; abort-before-dispatch captures never enter the canonical sent sequence
  • rebuild latest_context from canonical attempts and their observations, with malformed/old projections unable to block canonical append
  • guard cold projection repair with the canonical event-ledger revision so a stale scan cannot overwrite concurrently advanced authority
  • retire the current capture/attempt writer chain, duplicate DTOs and mappers, predictive request/prefix hashes, and live prompt-segment estimates while retaining historical readers and conversation-copy compatibility
  • fingerprint malformed compaction failures from the compactor's actual input rather than the retired predicted request shape

Lifecycle and authority

sequenceDiagram
    participant Runtime
    participant Seam as AI SDK model-call seam
    participant Artifact as Private artifact store
    participant Provider
    participant Ledger as Canonical ModelCallAttempt ledger
    participant Projection as Derived projections
    participant Diagnostics as Context diagnostics

    Runtime->>Runtime: Normalize one secret-free prepared request
    Note over Runtime: Derive serialization, digest, bytes, and segments together
    Runtime->>Artifact: Start optional serialized-request persistence
    Note over Runtime,Artifact: Best effort and never awaited by dispatch or accounting
    Runtime->>Runtime: Check abort and accounting readiness

    alt Aborted before seam invocation
        Runtime--xSeam: Do not invoke doStream or doGenerate
        Note over Ledger,Projection: No canonical attempt, sent entry, or projection update
    else AI SDK seam invoked
        Runtime->>Seam: doStream or doGenerate with prepared params
        Note over Runtime,Seam: Seam invocation authorizes the sent attempt
        Seam->>Provider: Provider-specific transport
        Note over Seam,Provider: Observation is semantic SDK input, not provider wire
        Provider-->>Runtime: Settlement and provider-reported usage
        Runtime->>Ledger: Commit canonical attempt with observation
        alt Projection is valid or absent
            Ledger->>Projection: Derive completed-main projection monotonically
        else Projection is malformed
            Ledger-->>Projection: Skip derived write and never roll back authority
        end
    end

    Diagnostics->>Projection: Read latest_context
    alt Current canonical v2 snapshot
        Projection-->>Diagnostics: Warm O(1) composition
    else Missing, old, or malformed
        Diagnostics->>Ledger: Read canonical ledger revision
        Diagnostics->>Ledger: Rebuild from canonical attempts
        Note over Ledger: Legacy provider events apply only if no canonical-era record exists
        Ledger-->>Diagnostics: Authority-derived candidate
        Diagnostics->>Ledger: CAS-check the same ledger revision
        alt Revision unchanged
            Diagnostics->>Projection: Repair derived row
        else Authority advanced concurrently
            Diagnostics--xProjection: Drop stale repair and do not write
        end
    end
Loading

The dependency is deliberately one-way: canonical events may derive or repair projections; projection absence, corruption, schema drift, or repair failure cannot block or rewrite canonical authority. RuntimeEvents and checkpoints remain the authority for conversation and compaction history. The observation only explains the semantic request Maka handed to the AI SDK seam.

Scope

This PR implements Workstream 1 of #4299 only. It does not implement semantic prefix continuity, continuity verdicts, provider-specific wire capture, or Inspector UI changes. Tool Availability remains owned by its existing runtime.

Relates to #4299.

Testing

  • full Storage suite: 1,039 passed, 8 skipped
  • focused Runtime and Runtime Host lifecycle/projection suite: 83 passed
  • Core, Storage, Runtime, and Runtime Host builds
  • Biome formatting and ASF license-header audit
  • adversarial review of observation security, lifecycle authority, projection upgrade/recovery, compaction compatibility, test contracts, and simplification

@Astro-Han
Astro-Han force-pushed the refactor/4299-request-observation-authority branch from 07565cc to 4c96ffa Compare August 31, 2026 03:58
@Astro-Han
Astro-Han marked this pull request as ready for review August 31, 2026 08:02

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the latest head e3df5b052. No P0 or P1 — approving. One P3 inline. All three checks are terminal green. This is 35 files across 10 commits, so I went after the authority and integrity fixes rather than reading evenly; scope is stated at the end.

Keeping projections subordinate to authority is the right principle and is implemented correctly. A malformed projection row now resolves to a 'malformed' state that callers handle, instead of throwing out of a read that authority could have satisfied. A derived cache must never be able to break a read the source of truth can serve, and the default path stays fail-loud rather than swallowing corruption everywhere.

The repair guard is the strongest part. Conditioning a derived repair on ifLedgerRevision makes it a compare-and-swap against the canonical ledger, so a repair computed from a stale read cannot overwrite state derived from newer authority. The test earns particular credit: it corrupts the projection row directly through raw SQL (SET event_json = '{malformed') and then verifies no cold answer is persisted once canonical authority advances. That drives the actual race rather than asserting the happy path.

Two real correctness catches in the digest work. Widening the telemetry cache key from ${step}:${requestHash} to include providerId and modelId closes a genuine collision: identical request shapes under different providers or models previously shared a key. And deriving requestHash from the actual serializedRequest instead of a separately built JSON.stringify([...]) makes the digest describe what is really sent — a digest computed over a different structure than the request is not a digest of that request.

One note on method. The intermediate commits pass a malformed: 'throw' | 'missing' option, but that is not what the head looks like — it resolved into the inspected.state discriminated result. I reviewed the head state; anyone reading commit-by-commit should know the option does not survive to the final tree.

Scope. I examined the two authority commits, the malformed-projection handling, the ledger-revision repair guard, and the digest and cache-key changes. I did not line-by-line review request-shape.ts (768 lines changed) or the prompt-composition and latest-context-snapshot rewrites. "Approved" here means no P0/P1 in the areas I drove, not that every line of a 2,136-addition change was verified.

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

replaceProjectionId || ledgerRevision
? {
...(replaceProjectionId ? { replaceEventId: replaceProjectionId } : {}),
...(ledgerRevision ? { ifLedgerRevision: ledgerRevision } : {}),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

P3 — this guard is silently inert when the optional method is absent.

readEventLedgerRevision is optional on the AgentRunStore type in core/src/agent-run.ts:798, and this call site spreads it conditionally:

...(ledgerRevision ? { ifLedgerRevision: ledgerRevision } : {})

So a store that does not implement the method yields undefined, the option is omitted, and the repair proceeds unguarded — which is precisely the race commit b60d9e717 exists to close. "No guard available" and "guard satisfied" become indistinguishable at this call site.

It is not reachable today, and I checked rather than assumed: agent-run-store.ts declares the method non-optionally on its own interface and implements it at line 677, and execution-stores.ts:501 forwards it. Both production stores provide it, so the guard always applies in practice.

The gap is that a safety property depends on an optional member with a silent fallback. If the intent is that repair must always be guarded, making the method required on the store type this path consumes — or failing loudly when it is missing rather than proceeding — would keep the guarantee from quietly evaporating. The same shape applies at history-compact-ledger.ts:134.

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

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants