test(ding): pin the Codex adapter's blocked predicate, and relocate harness tests - #37
Closed
schickling-assistant wants to merge 3 commits into
Closed
test(ding): pin the Codex adapter's blocked predicate, and relocate harness tests#37schickling-assistant wants to merge 3 commits into
schickling-assistant wants to merge 3 commits into
Conversation
The previous commit duplicated the progress-line shape into the Codex adapter and claimed the behaviour was unchanged. That claim rested on nothing: no Codex fixture contains a progress line, so the copied function was never called by the suite. Neutering it to `false` left all 144 tests green. A transcription slip in an unexercised copy would silently unblock a pane, which is the one direction this subsystem must never fail in. The test drives the four real active frames and the four real finished ones through a Codex composer: active leaves an empty composer unproven and a staged notice `ExactBlocked`, finished stays deliverable. Neutering the predicate now fails it. Also corrects the Codex spec's known limit, which claimed an unrecognized composer is `Ambiguous` — the same overclaim already fixed in the Claude spec. Under positional dispatch, failing to locate means this harness proves nothing about the screen; the screen is still offered to every other harness, and `Ambiguous` is what results when none of them locates a composer. agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
The ontology being added in #36 records `root` as the worst homograph in the system — six senses, four of them inside these VRS documents — and prescribes never writing it bare. This file, which the same series added, used it bare twice in one sentence. The referent here is the root VRS documents, which is the one sense the ontology does not enumerate as a directory, so the bare word invited exactly the wrong reading. agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
The split moved the code and left every test in `mod.rs`, so the module
boundary the trait establishes was invisible in the tests: 24 tests in
`mod.rs`, zero in `composer.rs`, `harness/claude.rs` and `harness/codex.rs`.
A reader looking for what pins the Codex adapter had no reason to look in the
file that defines it.
Six tests move to the module they exercise:
composer.rs maintained_composer_classifiers_require_exact_idle_state
transcript_composers_never_outrank_the_live_bottom_composer
composer_positions_are_compared_as_rows_not_raw_byte_offsets
stripping_preserves_newlines_for_well_formed_sequences_only
harness/claude.rs an_in_flight_turn_blocks_return_but_a_finished_one_does_not
harness/codex.rs the_codex_adapter_still_blocks_on_the_inherited_progress_line
`maintained_composer_classifiers_require_exact_idle_state` asserts across seven
Claude fixtures and three Codex ones in a single test, so it belongs with the
router rather than either harness — and splitting it would divide one
INVARIANTS-named contract into two handles, which is not this commit's to do.
The screen fixtures move to a new `#[cfg(test)] mod fixtures`. They have
consumers in all four modules — the router tests, the transport tests, and both
adapters assert against the same shapes — so they are shared rather than
duplicated per consumer, where two copies could drift and quietly weaken
whichever test kept the stale one. `msg` stays in `mod.rs`: it builds a
Message, not a screen, and only the transport tests use it.
`INVARIANTS.md` repoints its one affected reference to
`src/ding/composer.rs::maintained_composer_classifiers_require_exact_idle_state`.
The other five movers are not INVARIANTS-named.
Pure move. No test renamed, no assertion changed, no coverage added. The one
edit inside a moved body is a path qualifier the move invalidates:
`composer::strip_ansi` becomes `strip_ansi`, the same function, now in scope
because the test lives in the module that defines it.
agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d
agent-tool: Claude Code
agent-tool-version: 2.1.220
agent-model: claude-opus-5
agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
schickling-assistant
marked this pull request as ready for review
July 28, 2026 15:31
Contributor
|
Closing per disposition: superseded by current stable main; the remaining conflict is fresh-DND-only and is intentionally handled on main. |
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.
Follow-up to #27. Adds the test that pins the Codex adapter's blocked-state predicate, and corrects one overclaim in its spec.
Why this is worth a PR of its own
maincurrently carries a duplicated safety predicate that no test exercises.The per-harness split gave each harness its own blocked-state detection, so the Codex adapter got its own copy of the progress-line predicate rather than calling into the Claude adapter — that coupling is what decision 0001 forbids. The copy was made on the grounds that behaviour was unchanged. That claim rested on nothing: no Codex fixture contains a progress line, so the copied function is never called by the suite. Neutering it to
return falseleaves all tests green.That is the wrong direction to be untested in. This predicate decides whether a pane is blocked; a transcription slip in the copy silently unblocks a pane, which is the one failure mode the subsystem is built to prevent. Every other blocked-state signal here is pinned by a test.
What this adds
AmbiguoustoEmptySafeand fails the test.Spec correction
02-codex/spec.mdrepeated an overclaim already fixed in the Claude spec: that a screen whose composer cannot be located isAmbiguous. Under positional dispatch that is false. Failing to locate means this harness proves nothing about the screen; every other maintained harness is still offered it, andAmbiguousis the result only when none of them locates a composer.Gates
On top of
a77776ac:cargo test --lib --bins145 passed / 0 failed;nix flake checkexit 0;axe vrs check --profile strict docs/vrsok.Still outstanding, not addressed here
The Codex adapter's blocked predicate is split structurally but still holds the undivided set inherited from the shared predicate, including shapes only the other harness renders. That over-blocks rather than under-blocks, so it is consistent with the subsystem's fail-closed tradeoff, and it is recorded as a known limit in the spec rather than described as done. Narrowing it to this harness's own shapes is separate work.
Why a test PR carries a one-line doc edit
4e75c7bqualifies two bare uses of root in01-ding/requirements.md. The ontology in #36 recordsrootas the system's worst homograph — six senses, four of them inside these VRS documents — and prescribes never writing it bare. That sentence's referent is the root VRS documents, which is the one sense the ontology does not enumerate as a directory, so the bare word invited exactly the wrong reading. It would be poor form for the series introducing that rule to ship a sibling document breaking it.Relocating the harness tests
The split that landed in #27 moved the code and left every test behind: 24 in
src/ding/mod.rs, zero incomposer.rs,harness/claude.rsandharness/codex.rs. The module boundary the trait establishes was invisible in the tests, and a reader looking for what pins the Codex adapter had no reason to open the file that defines it. The test added above landed inmod.rsfor the same reason, so it is moved here rather than in a follow-up.Six tests move to the module they exercise:
composer.rsmaintained_composer_classifiers_require_exact_idle_statecomposer.rstranscript_composers_never_outrank_the_live_bottom_composercomposer.rscomposer_positions_are_compared_as_rows_not_raw_byte_offsetscomposer.rsstripping_preserves_newlines_for_well_formed_sequences_onlyharness/claude.rsan_in_flight_turn_blocks_return_but_a_finished_one_does_notharness/codex.rsthe_codex_adapter_still_blocks_on_the_inherited_progress_linemaintained_composer_classifiers_require_exact_idle_stateasserts across seven Claude fixtures and three Codex ones in one test, so it belongs with the router rather than either harness. Splitting it would divide one INVARIANTS-named contract into two handles.Transport, staging, observation, FIFO, backoff, watch and inbox tests stay in
mod.rs— they test delivery, not a harness.Fixtures
The screen builders move to a new
#[cfg(test)] mod fixtures. They have consumers in all four modules — the router tests, the transport tests, and both adapters assert against the same shapes — so they are shared rather than duplicated per consumer, where two copies could drift and quietly weaken whichever test kept the stale one.msgstays inmod.rs: it builds a Message, not a screen, and only the transport tests use it.That this is a pure move
cargo test --lib -- --listleaf names are byte-identical across4e75c7b→268833c— 145 names, none renamed, none added, none removed.pub(crate)visibility. The single edit inside a moved body is a path qualifier the move invalidates:composer::strip_ansibecomesstrip_ansi, the same function, now in scope because the test lives in the module that defines it.Gates
cargo test --lib --bins145 passed / 0 failed;nix flake checkexit 0. All 27src/<file>.rs::<test>references inINVARIANTS.mdresolve, 0 unresolved — one was repointed, tosrc/ding/composer.rs::maintained_composer_classifiers_require_exact_idle_state; the other five movers are not INVARIANTS-named.Per-file
#[test]counts after:mod.rs18,composer.rs4,harness/claude.rs1,harness/codex.rs1,harness/mod.rs0,fixtures.rs0.Posted on behalf of @schickling
agent_nameagent_session_idagent_toolagent_tool_versionagent_runtimeagent_modelruntime_profileskills_manifestworktreemachinetooling_profile