fix(delivery): eliminate the fresh-mode eager-bundling regression (#323 T1/T2/T4) - #326
Merged
Conversation
… T1/T2/T4) Eager resource bundling (c906b3a) regressed fresh-mode delivery by +24.5% on get_activity and +8.3% overall (#322). Two defects, plus a gate that could not see either. T1 — gate the resources map on referenceMode. The collapse check could never fire under full delivery, so every linked body shipped in full in every activity that linked it, with no later delivery to collapse against. Under full delivery the map is no longer built: the linked ids go out as resource_refs, no resource:<id> ledger keys are written (they were written and never read), and no resource_fetched{bundled} event claims a delivery that did not happen. T2 — bring resource bodies inside the eager budget. They now draw down the same cumulative spentChars counter as technique bodies, with the technique loop's stop-and-break semantics, so context_tokens actually bounds the response. Overflowed and oversized ids join resource_refs, so nothing linked becomes unreachable. The 80,000-char per-resource guard is retained as a secondary check that skips one resource without stopping the loop. T4 — add a fresh-mode arm to the ship gate. The A0 fixture now records its contextMode and corpus revision; a cross-mode comparison is banner-warned and can never pass --gate, which is the hole the July gate fell through. --gate [--max-regression-pct=N] exits 3 on a total-delivery-char regression. resources_note becomes the single authority on how a response delivered its linked resources; step_techniques_note points at it rather than restating it. Measured on corpus a1409d5, fresh mode (A0 = 1,355,532 chars): before get_activity 856,425 (+24.5%) total 1,467,485 (+8.3%) after get_activity 692,377 (+0.6%) total 1,359,973 (+0.3%) Persistent mode is preserved: get_resource chars, resource:* ledger keys and unchanged-resource answers are unchanged; get_activity moves only by the note rewording (516,898 -> 516,091). T3 (per-worker ledger keying) is deliberately not included: it requires changes to the workflows corpus and an ADR, and the issue sequences it as a separate PR. Refs #322 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mike Clay <mike.clay@shielded.io>
…hots
Twelve tests asserted on corpus content that has since moved. Each is fixed at
the assertion, not by loosening it — the behaviour under test is still covered.
mcp-server (4) — three cross-workflow get_resource tests fetched
meta/activity-worker-prompt, a resource deleted from the corpus; they now fetch
meta/bootstrap-protocol. The condition_not_met rejection test yielded
analysis-confirmed, a checkpoint that no longer exists, so yield_checkpoint
errored before the assertion; it now uses validate::local-validation-permission,
verified unconditional (the rejection path needs a checkpoint declaring no
`condition`, and the sibling test's issue-verification is conditional).
reference-delivery (3) — the mutation test anchored on the variable-binding
capability's wording ("Bind a step"); it now anchors on the `## Capability`
heading, which is structural and does not drift. The other two asserted an
activity_rules block on work-package, which declares only orchestrator-scoped
rules.workflow and therefore carries none: the shape test now asserts its
absence, and the collapse behaviour moved to a new test against
requirements-refinement, which declares both rules.activity and rules.universal.
The ledger-key-shape test moved to the same workflow so all three key shapes,
activity_rules:* included, are exercised by one walk.
fragment-resolver (2) — materialization was proven by a transcribed sentence from
the fragment body. It now reads the declared body out of workflow.yaml and
asserts the loader inlined that, so the test proves materialization without
re-encoding corpus prose.
e2e snapshots (6) — regenerated. The committed baselines predate several corpus
releases (the analyse-challenge refactor, portfolio-lens removal, model-switch
checkpoint removal). Regenerated against the corpus revision this repo records
as its submodule pointer, and verified byte-identical against the pristine
pointer — so the update is independent of the corpus-guard branch.
Whole suite: 701 passed, 0 failed. The fresh-mode delivery gate still passes at
+0.3% vs A0.
Signed-off-by: Mike Clay <mike.clay@shielded.io>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Companion corpus PR: #325 — it fixes the three corpus guard defects that leave fragments-guard, identifier-qualification and resource-anchors red here (all three were already red on main; this PR takes the suite from 18 failures to 3). Merge #325 first, then bump the workflows submodule pointer on this branch for a fully green suite. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the T1/T2/T4 half of #323 — the fresh-mode eager-bundling regression diagnosed in #322 — and unsticks the corpus-coupled tests that were red before it.
T3 is deliberately not here. It needs a design decision recorded as an ADR, and its acceptance criterion requires relaxing
workers-need-full-delivery, which lives in the workflows corpus — so unlike T1/T2/T4 it is not a server-only change. #323 sequences it as a separate PR, and it is where the remaining ≈96k tokens/walk live.T1 — gate the
resourcesmap onreferenceModeThe collapse check was
referenceMode && (deliveredHash(…) === hash || …), so under full delivery it could never fire: every linked resource body shipped in full, in every activity that linked it, with no later delivery to collapse against.Under full delivery the map is no longer built. The linked ids go out as
resource_refsinstead, so the worker still learns exactly what to fetch (exactresource_id,#sectionincluded). Noresource:<id>ledger key is written — those were written and never read — and noresource_fetched { bundled: true }history event claims a delivery that never happened.T2 — bring resource bodies inside the eager budget
The technique loop honoured
eagerBudgetChars; the resource loop had only a per-resource 80,000-char skip, no cumulative cap and no draw-down. An activity linking twenty 5KB resources inlined 100KB unbounded, socontext_tokensdid not bound the response — the stated purpose of the budget policy insrc/config.ts.Resource bodies now draw down the same
spentCharscounter as technique bodies, with the technique loop's stop-and-break semantics. Overflowed and oversized ids joinresource_refs, so nothing linked becomes unreachable. Unchanged-reference markers stay free and never displace a body that still needs sending.T4 — a fresh-mode arm on the ship gate
The July gate compared A0 (fresh, pre-change) against A3 (persistent, post-change), conflating a mode switch with a code change — its own ablation attributed −40.2% to the mode switch alone, and a +24.5% regression on the only mode production uses was invisible to it.
The A0 fixture now records its
contextModeand corpus revision. A cross-mode comparison still reports but is banner-warned and can never pass--gate(verified: exit 3 even at--max-regression-pct=99).--gate [--max-regression-pct=N]exits 3 on a total-delivery-char regression; a corpus mismatch against the fixture'sworkflowsRevis warned in the scorecard.Measurements
Corpus pinned at
a1409d5b,context_mode: fresh, A0 = 1,355,532 chars:get_activity#323's falsifier — "if fresh-mode total is not within ~1% of 1,355,532, the attribution in #322 is wrong" — does not fire. The residual ≈4.4k on
get_activityis 12 calls × theresource_refslist plus the reworded notes.Persistent mode is preserved:
get_resourcechars (73,683),resource:*ledger keys (38) and unchanged-resource answers (63) are all identical;get_activitymoves only by the note rewording (516,898 → 516,091).Test fixes (second commit)
Twelve tests asserted on corpus content that has since moved. Each is fixed at the assertion, not loosened.
meta/activity-worker-prompt, deleted from the corpus →meta/bootstrap-protocol. Thecondition_not_metrejection test yieldedanalysis-confirmed, which no longer exists, soyield_checkpointerrored before the assertion →validate::local-validation-permission, verified unconditional (the rejection path needs a checkpoint declaring nocondition; the sibling test'sissue-verificationis conditional and could not be reused).variable-bindingcapability's wording; it now anchors on the## Capabilityheading, which is structural. The other two expected anactivity_rulesblock onwork-package, which declares only orchestrator-scopedrules.workflowand carries none: the shape test now asserts its absence, and the collapse behaviour moved to a new test againstrequirements-refinement, which declares bothrules.activityandrules.universal. The ledger-key-shape test moved to the same workflow so one walk exercises all three key shapes,activity_rules:*included.workflow.yamland asserts the loader inlined that, so it proves materialization without re-encoding corpus prose.Known-failing until the corpus PR lands
Three corpus guard tests (
fragments-guard,identifier-qualification,resource-anchors) fail here. They were already red onmain— this PR takes the suite from 18 failures to 3 — and the defects are in the corpus, not the server. The companion corpus PR fixes all three. The submodule pointer is deliberately left untouched here so this PR does not smuggle in 23 commits of unrelated corpus evolution; bump it once the corpus PR merges and the suite is fully green.Docs
docs/resource_resolution_model.md§12 is the authoritative home for the delivery contract and states the two shapes and the budget;docs/development.mddocuments--gateand, explicitly, that a persistent-only comparison is not a valid ship gate.Refs #322, #323
🤖 Generated with Claude Code