refactor(speculation)!: drop the ignored dependency assumption - #558
Merged
Conversation
## Summary ### Why? `DependencyAssumptionIgnored` was vocabulary the system could not produce. It expressed conflict relaxation — "this path makes no claim about this dependency" — but nothing ever set it: `bestfirst` only ever writes *succeeds* or *fails*, and no other Speculator exists. Outside the entity definition its only non-test appearance was `isWellFormed` accepting it as a legal value. Dead vocabulary is not free. Every consumer that reads an assumption had to decide what a third value meant, and each one answered by omission — `assumptionBroken`, `allAssumedSucceedingMerged` and `Base` all silently skip it, in three separate switches with no case tying them together. That is the kind of thing that stays correct only as long as nobody adds a fourth reader. It also cost real reasoning: while reviewing the CODEM-424 fix, `ignored` was the state that made a merged head's stored path impossible for the generator to regenerate, and working out whether that was reachable took a probe to answer, for a value nothing emits. ### What? The constant is gone, along with `isWellFormed`'s acceptance of it and every doc comment describing it. An assumption is now total: *succeeds* or *fails*, one per dependency. Test fixtures used `ignored` heavily as a "don't care" filler for the second dependency. On a dependency that has not finished, *fails* is indistinguishable from it — neither breaks the path nor gates the merge — so those became *fails*. The fixtures that could not be substituted mechanically are the ones where every dependency is resolved: exactly one assumption pair is unbroken there, so tests needing two live paths now share that shape and separate the entries by ID, which the existing helpers already supported. `speculation.md`'s relaxation section is rewritten rather than deleted, so the design intent survives the removal. It now records that relaxation is unimplemented, and that when it is built it belongs in the controller as a trim of the dependency list handed to the Speculator — keeping the decision beside the other correctness decisions, and keeping a path a total function over the dependencies it was given. It also records the question that design still owes an answer to: a path built against a trimmed list no longer lines up with a head whose list grew back, and `isWellFormed` rejects it, so the trim has to be stable for a head's lifetime or recorded with the path. **Breaking:** any persisted path carrying `ignored` fails `isWellFormed` after this and is dropped as malformed. Nothing writes the value, so no such record should exist. ## Test Plan - ✅ `make test` — 96/96 pass - ✅ `make lint`, `make check-gazelle`, `make check-tidy` The removal is covered by what the fixtures no longer need: `TestAssumptionBroken` and `TestMergeablePath` lose their ignored-specific rows, and `TestIsWellFormed` now rejects the value along with every other non-assumption. ## Issue Groundwork for https://linear.app/uber/issue/CODEM-424 and https://linear.app/uber/issue/CODEM-429, both of which had to reason about a value nothing produces.
behinddwalls
marked this pull request as ready for review
August 10, 2026 18:40
behinddwalls
marked this pull request as draft
August 10, 2026 18:46
behinddwalls
marked this pull request as ready for review
August 10, 2026 19:04
mnoah1
approved these changes
Aug 10, 2026
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.
Summary
Why?
DependencyAssumptionIgnoredwas vocabulary the system could not produce. It expressed conflict relaxation — "this path makes no claim about this dependency" — but nothing ever set it:bestfirstonly ever writes succeeds or fails, and no other Speculator exists. Outside the entity definition its only non-test appearance wasisWellFormedaccepting it as a legal value.Dead vocabulary is not free. Every consumer that reads an assumption had to decide what a third value meant, and each one answered by omission —
assumptionBroken,allAssumedSucceedingMergedandBaseall silently skip it, in three separate switches with no case tying them together. That is the kind of thing that stays correct only as long as nobody adds a fourth reader. It also cost real reasoning: while reviewing the CODEM-424 fix,ignoredwas the state that made a merged head's stored path impossible for the generator to regenerate, and working out whether that was reachable took a probe to answer, for a value nothing emits.What?
The constant is gone, along with
isWellFormed's acceptance of it and every doc comment describing it. An assumption is now total: succeeds or fails, one per dependency.Test fixtures used
ignoredheavily as a "don't care" filler for the second dependency. On a dependency that has not finished, fails is indistinguishable from it — neither breaks the path nor gates the merge — so those became fails. The fixtures that could not be substituted mechanically are the ones where every dependency is resolved: exactly one assumption pair is unbroken there, so tests needing two live paths now share that shape and separate the entries by ID, which the existing helpers already supported.speculation.md's relaxation section is rewritten rather than deleted, so the design intent survives the removal. It now records that relaxation is unimplemented, and that when it is built it belongs in the controller as a trim of the dependency list handed to the Speculator — keeping the decision beside the other correctness decisions, and keeping a path a total function over the dependencies it was given. It also records the question that design still owes an answer to: a path built against a trimmed list no longer lines up with a head whose list grew back, andisWellFormedrejects it, so the trim has to be stable for a head's lifetime or recorded with the path.Breaking: any persisted path carrying
ignoredfailsisWellFormedafter this and is dropped as malformed. Nothing writes the value, so no such record should exist.Test Plan
make test— 96/96 passmake lint,make check-gazelle,make check-tidyThe removal is covered by what the fixtures no longer need:
TestAssumptionBrokenandTestMergeablePathlose their ignored-specific rows, andTestIsWellFormednow rejects the value along with every other non-assumption.Issue
Groundwork for https://linear.app/uber/issue/CODEM-424 and https://linear.app/uber/issue/CODEM-429, both of which had to reason about a value nothing produces.