docs(stella-parity): stop the calibration row claiming a wiring two paths do not have (#1595) - #1924
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the stella-parity capabilities documentation for calibration drift to accurately describe which CLI paths seed the CalibrationMap and clarifies why the capability remains marked as ShippedUnwitnessed, without changing any behavior or wiring. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…aths do not have The `calibration.drift` capability declared its CLI mechanism as "seed_calibration from the store plus with_calibration on **every driver path**". That is false. Five paths seed it — interactive, raw one-shot, goal, the Command Deck and deck sub-sessions — and two do not: - `run_pipeline_one_shot`, which is the **default `stella run` path** - fleet workers (`fleet_cmd.rs`) So the engines the staged pipeline drives run with an un-seeded estimator while the matrix says otherwise (#1595). A capability matrix exists so a divergence is *declared rather than assumed* (AGENTS.md invariant 8's discipline, applied across surfaces instead of across providers). A row that is confidently wrong is worse than a row marked unwitnessed: it is the thing a reviewer checks *instead of* checking the code. This corrects the mechanism to name which paths seed and which do not, and sharpens `missing` to say that the absent CLI-side witness is exactly what would have caught the two gaps — the row was unwitnessed and wrong at the same time, and those two facts are the same fact. `UNWITNESSED_BASELINE` is unchanged at 3: the posture is still `ShippedUnwitnessed`, because it still is. Promoting it to `Shipped` is what #1595 closes, and doing that before the two paths are wired would re-tell the same lie in a stronger tense. ## What this deliberately does not do Thread the calibration map into the pipeline. That is #1595's option 1, it is the right answer, and it is a larger change than this — so it stays open with the design recorded on the issue rather than half-landed here. #1595's option 2 (declare the exemption as deliberate) is NOT taken: there is no principled reason the pipeline's engines should estimate worse than the deck's, so documenting it as intentional would be filing a bug as a feature. The investigation #1595 asks for up front — "establish whether the pipeline's engine construction can consume a `CalibrationMap` at all" — is answered: it can, and the shape is constrained. `CalibrationMap` derives `Debug, Default` but **not `Clone`**, while `PipelineConfig` derives `Clone`, so the map cannot be an owned config field; and its own doc comment states the intended ownership — "the caller owns the map across turns and hands the engine a shared reference, mirroring how `BudgetGuard` outlives individual turns". The correct home is therefore a borrowed field on `Pipeline` with a `with_calibration` builder mirroring the existing `with_gate`, consumed at the four `Engine::with_sleeper` sites. Full write-up on the issue. `cargo test -p stella-parity` — 8 passed. Clippy `-D warnings` and `fmt --check` clean. Refs #1595, #1557
macanderson
force-pushed
the
fix/1595-pipeline-calibration
branch
from
August 6, 2026 22:31
f648fd8 to
c6fe959
Compare
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.
docs(stella-parity): stop the calibration row claiming a wiring two paths do not have
The
calibration.driftcapability declared its CLI mechanism as"seed_calibration from the store plus with_calibration on every driver
path". That is false. Five paths seed it — interactive, raw one-shot, goal,
the Command Deck and deck sub-sessions — and two do not:
run_pipeline_one_shot, which is the defaultstella runpathfleet_cmd.rs)So the engines the staged pipeline drives run with an un-seeded estimator
while the matrix says otherwise (#1595).
A capability matrix exists so a divergence is declared rather than assumed
(AGENTS.md invariant 8's discipline, applied across surfaces instead of across
providers). A row that is confidently wrong is worse than a row marked
unwitnessed: it is the thing a reviewer checks instead of checking the code.
This corrects the mechanism to name which paths seed and which do not, and
sharpens
missingto say that the absent CLI-side witness is exactly whatwould have caught the two gaps — the row was unwitnessed and wrong at the same
time, and those two facts are the same fact.
UNWITNESSED_BASELINEis unchanged at 3: the posture is stillShippedUnwitnessed, because it still is. Promoting it toShippedis what#1595 closes, and doing that before the two paths are wired would re-tell the
same lie in a stronger tense.
What this deliberately does not do
Thread the calibration map into the pipeline. That is #1595's option 1, it is
the right answer, and it is a larger change than this — so it stays open with
the design recorded on the issue rather than half-landed here. #1595's option 2
(declare the exemption as deliberate) is NOT taken: there is no principled
reason the pipeline's engines should estimate worse than the deck's, so
documenting it as intentional would be filing a bug as a feature.
The investigation #1595 asks for up front — "establish whether the pipeline's
engine construction can consume a
CalibrationMapat all" — is answered:it can, and the shape is constrained.
CalibrationMapderivesDebug, Defaultbut not
Clone, whilePipelineConfigderivesClone, so the map cannotbe an owned config field; and its own doc comment states the intended
ownership — "the caller owns the map across turns and hands the engine a
shared reference, mirroring how
BudgetGuardoutlives individual turns". Thecorrect home is therefore a borrowed field on
Pipelinewith awith_calibrationbuilder mirroring the existing
with_gate, consumed at the fourEngine::with_sleepersites. Full write-up on the issue.cargo test -p stella-parity— 8 passed. Clippy-D warningsandfmt --checkclean.Refs #1595, #1557
Summary by Sourcery
Documentation: