Skip to content

refactor(speculation)!: drop the ignored dependency assumption - #558

Merged
mnoah1 merged 1 commit into
mainfrom
preetam/drop-ignored-assumption
Aug 10, 2026
Merged

refactor(speculation)!: drop the ignored dependency assumption#558
mnoah1 merged 1 commit into
mainfrom
preetam/drop-ignored-assumption

Conversation

@behinddwalls

Copy link
Copy Markdown
Collaborator

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.

## 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
behinddwalls marked this pull request as ready for review August 10, 2026 18:40
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners August 10, 2026 18:40
@behinddwalls
behinddwalls marked this pull request as draft August 10, 2026 18:46
@behinddwalls
behinddwalls marked this pull request as ready for review August 10, 2026 19:04
@mnoah1
mnoah1 added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 3e20009 Aug 10, 2026
39 checks passed
@mnoah1
mnoah1 deployed to stack-rebase August 10, 2026 20:12 — with GitHub Actions Active
@behinddwalls
behinddwalls deleted the preetam/drop-ignored-assumption branch August 10, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants