Skip to content

Re-express the legacy CS hypercube invariants against current models - #2275

Open
sei-ahouseholder wants to merge 6 commits into
mainfrom
task/2237-cs-hypercube-invariants
Open

Re-express the legacy CS hypercube invariants against current models#2275
sei-ahouseholder wants to merge 6 commits into
mainfrom
task/2237-cs-hypercube-invariants

Conversation

@sei-ahouseholder

@sei-ahouseholder sei-ahouseholder commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Mines the legacy vultron/core/case_states/ hypercube for still-valid CS
invariants and re-expresses the survivors against the current enum models, with
the legacy module's status decided and recorded. Also fixes the global 5s
per-test timeout that was aborting the integration suite.

Changes

  • vultron/core/states/cs_invariants.py (new): the CS validity library.
    Compound state validity (the 32 CS members are the valid states; vF and
    fD are structurally impossible), transition validity that delegates to the
    per-dimension transition tables
    so it cannot drift from what VfdDimension /
    PxaDimension enforce, the two ephemeral-state rules (vP requires V next,
    pX requires P next), and history validity by causal replay. Every entry
    point accepts either CSEvent members or the legacy single-letter strings.
  • test/core/states/test_cs_invariants.py (new, 110 tests): ratchets the
    re-expression against an independent derivation of the legacy string-pattern
    rules — 32 states, 58 transitions (from 72 before the ephemeral rule), 70 of
    720 permutations, and agreement with the legacy oracle on every
    permutation in both input forms. Slowest test 0.07s.
  • docs/adr/0060-...md (new): the legacy module is kept as a demoted
    reference model
    , not retired. Its two named retirement prerequisites are
    recorded.
  • specs/cs-behavior.yaml: adds group CSB-17 (5 specs), extending CSB-13,
    constraining CSB-16, and refining SM-09-001/002.
  • specs/state-machine.yaml: SM-09-001's rationale repointed from the legacy
    module at cs_invariants.py, and scoped explicitly to the persistence boundary.
  • test/conftest.py, test/test_integration_timeout_tier.py (new, 16
    tests), test/AGENTS.md: two-tier per-test timeout (Integration suite aborts on the global 5s thread timeout, masking the merge signal #2270).
  • notes/case-state-model.md, notes/codebase-structure.md,
    notes/documentation-strategy.md, notes/flaky-tests.md,
    docs/reference/glossary.md: correct stale 64-state and
    vultron/case_states/ claims.
  • AGENTS.md: adds a Case States entry to the codebase map that routes
    new protocol-path work to cs_invariants.py and records what the legacy tree
    is still for (oracle in the equivalence tests; two live importers). Also
    relocates a dangling ISSUE-1784 reference to the rebase bullet it belongs to.

Two findings that changed the shape of the work

The issue's "completely orphaned" premise was wrong. There are two live
importers outside the legacy tree: vultron/core/use_cases/query/action_rules.py
(reached from the live actors_get_action_rules endpoint) and
vultron/core/states/cs.py:26, which imports validations.ensure_valid_state
the authoritative module depends on the legacy one. Retirement is therefore a
migration, not a deletion, which is why ADR-0060 chose "keep, demoted" over the
archive the issue anticipated.

Causal replay over index comparison. The legacy is_valid_history compares
event indices, which only works on a complete six-event history. Replaying
through is_valid_cs_transition is provably equivalent on complete histories
(asserted directly, over all 720 permutations) and additionally generalises to
prefixes — which is what real, in-progress cases actually are. CSB-17-005
pins the prefix rule in both directions: the accepted set is exactly the
prefix-closure of the 70 valid histories, so no accepted prefix is a dead end.

Reconciling with SM-09-001

The vP → VP and pX → PX rules were already a pre-existing MUST (SM-09-001),
which requires promotion before persisting. CSB-17-003 restates the same two
conditions as trajectory-validation rules, where an ephemeral state is a legal
intermediate point, and CSB-17-005 lets a history prefix end in one. The two are
consistent because they bind at different boundaries; SM-09-001's rationale and
ADR-0060 now say so explicitly rather than claiming the rule was "covered by no
spec."

The timeout fix (#2270)

uv run pytest -m integration exited 1 on clean origin/main with no summary
line
: several integration tests do 3.5-4.3s of honest work against a 5s
ceiling, and timeout_method = "thread" kills the whole pytest process rather
than the one slow test. A red integration run therefore carried no information
about the branch.

Timeouts are now two-tier, both sized from measurement:

Tier Was Now Slowest honest test
Unit (default) 5s 30s ~3.1s (test_real_specs_load setup)
@pytest.mark.integration 5s 60s ~4.3s (test_decision_audit_inventory)

Explicit @pytest.mark.timeout(N) always wins, so the demo suite's deliberate
values (10, 180) are untouched. Regression tests pin both tiers, the
timeout_method they depend on, and — via a real nested pytest session — the
timeout each item actually resolves to, which is the only way to catch a
hook-ordering regression: the marker is added at collection time, but
pytest-timeout reads it in pytest_runtest_protocol.

Three judgment calls worth flagging:

  • Widened the ceiling rather than switching timeout_method off thread.
    The signal method cannot interrupt code blocked in a C extension, so it would
    convert a noisy abort into an invisible hang.
  • Raised the unit tier too, which is beyond the reported failure. The unit
    suite has AST-walking architecture ratchets at ~3.4s against the same 5s
    ceiling. Three earlier learning files (ISSUE-1925, ISSUE-1988, ISSUE-2086)
    each documented this root cause and worked around it; fixing only the
    integration tier would have left the pattern intact. An intermediate value of
    20s was tried and still tripped once while a background graphify rebuild
    competed for CPU, hence 30s.
  • 30s is not proof against arbitrary CPU starvation. While verifying this
    branch, two full-suite runs aborted at the 30s unit ceiling with a 1-minute
    load average near 30 on 20 cores (a background graphify rebuild). The same
    suite passed twice at 116s with the machine idle. The tier values are sized
    against honest test duration, not against an oversubscribed host; a red run
    with no summary line is still worth checking loadavg against before
    diagnosing it as a code failure.

Cross-machine rules handed to #2236

Per the issue's scope, the cross-machine rules were fed to #2236 rather than
implemented here — see this comment.
The substantive finding: the legacy tree contains no RM rules at all, so
#2236's headline entailment (F implies RM in {ACCEPTED, DEFERRED, CLOSED})
is not derivable from the hypercube and must come from rm_em_cs.md. It does
contain a genuine CS -> EM table (patterns/embargo.py), and five advisory
tables that must not become emit guards.

Verification

Note for reviewers

Running -m integration locally writes devlogs/, which un-skips
test/ci/invariants/test_fv_invariants.py in the unit suite and produces 4
failures unrelated to this branch. Both modes are already tracked in #2273;
rm -rf devlogs/ restores green. This PR makes that interaction more likely by
letting the integration suite finish, which is noted on #2273.

…out tier

Mines vultron/core/case_states/ for still-valid CS invariants and re-expresses
the survivors against the current enum models, per issue #2237.

Adds vultron/core/states/cs_invariants.py: compound state validity, transition
validity delegating to the per-dimension tables, the two ephemeral-state rules
(vP requires V next, pX requires P next), and causal history validation by
replay — which generalises to in-progress prefixes where the legacy index
comparison could not.

The legacy module is kept as a demoted reference model rather than retired: it
has two live importers, including vultron/core/states/cs.py itself, so
retirement is a migration. Recorded as ADR-0060 with two named prerequisites.

Also fixes #2270: the global 5s per-test timeout fired on honest 3.5-4.3s
integration work and, because timeout_method is "thread", aborted the whole
pytest process with no summary line. integration-marked tests now get a 60s
tier while the unit suite keeps 5s.

Closes #2237
Fixes #2270

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Widening only the integration tier was an incomplete fix for #2270. The unit
suite has AST-walking architecture ratchets at ~3.4s against the same 5s
ceiling, so it had a load-dependent margin too — documented but never fixed in
three earlier learning files (ISSUE-1925, ISSUE-1988, ISSUE-2086).

Sized from measurement: slowest unit test is ~3.1s idle. An intermediate value
of 20s was tried and still tripped once while a background graphify rebuild
competed for CPU, so 30s. A generous ceiling costs nothing on a genuine hang —
that test was never going to finish — and total suite runtime (~110s) is bounded
by the tests, not by this ceiling.

Adds regression tests pinning both tiers and the timeout_method they depend on,
so a silent revert fails loudly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sei-ahouseholder

Copy link
Copy Markdown
Contributor Author

CI triage — the two red jobs are pre-existing, not from this branch

Everything that gates code quality is green: Lint (black/flake8/mypy/pyright), Tests (pytest), Spec Lint, Build, docs-build-check, CodeQL, Analyze (actions/python).

Red on this PR:

Job Also red on main?
fcvcv Demo Integration yes
fvcv-handoff Demo Integration yes
fvcv-handoff Invariant Harness yes

Comparison base: main run 31632495576 at 65fe33f1, where 6 of 9 demo scenarios failed — a superset of the 2 that failed here. This PR's run selected only 4 scenarios (fcv-reject, fcvcv, fv, fvcv-handoff); the 2 red ones are both in main's red set. Demo Integration has been red on main continuously since at least 2026-08-10 (8 consecutive runs), tracked by #2136 and #2203.

Failure signatures match. fcvcv on this PR records the same five checks as main:

  • CaseActor reflects .../vendor at RM.ACCEPTED (timeout; current=RM.VALID)
  • CaseActor reflects .../vendor-deployer at RM.ACCEPTED (timeout; current=RM.VALID)
  • M5: C1 replica shows V1 and V2 CS include F (timeout; vfd_state still vfd)
  • M6: C1 replica shows V1=VFd, V2=VFD
  • M7: all replicas EM.EXITED, all participants public-aware

plus one extra: Embargo terminated (EM.EXITED) after V1 reports published — the wait_for_case_em_terminated async-race timeout catalogued in #1978. Recorded-failure counts on main itself swing run to run (7 at 65fe33f1, 2 at 1f930bd6), so 8-vs-7 is within observed variance rather than a signal.

Causality: the Demo Integration jobs run vultron-demo under docker-compose and invoke no pytest, so this branch's pyproject.toml / test/conftest.py timeout changes cannot reach them. The Invariant Harness job does run pytest, but the change only widens ceilings, and fvcv-handoff Invariant Harness is red on main too. Nothing in this diff touches RM, EM, or demo code — vultron/core/states/cs_invariants.py is a new module with no production callers, and vultron/core/states/__init__.py gains re-exports only.

@sei-ahouseholder sei-ahouseholder left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Triage: #2275 — Re-express the legacy CS hypercube invariants against current models

Linked issues: #2237 (mine the legacy case-states hypercube), #2270 (integration suite aborts on the 5s thread timeout)
Changed files: 18 files — core/states, test-harness, specs/, docs/adr/, notes/, plan/learnings
CI status: ❌ failing — 3 jobs, all pre-existing and tracked (see below)
Merge state: ✅ mergeable (UNSTABLE — no conflicts; the red non-required checks are the cause)
Base branch: main
Needs integration tests: yes


First: the three red jobs are not this PR

fcvcv Demo Integration, fvcv-handoff Demo Integration, fvcv-handoff Invariant Harness all trace to one deterministic defect — the invite-path engage-case trigger returning HTTP 422 (SvcEngageCaseUseCase failed: TransitionParticipantRMtoAccepted). Root cause #2233; harness-side assertion scope #2266.

Evidence: main run 31632495576 (65fe33f1) fails all three identically; sibling branches bug/2239 and bug/2235 do too; so does this branch's own prior commit 527200bb. Both "the timeout change did it" hypotheses were tested and refuted — the demo jobs run docker compose + the demo CLI and never invoke pytest, and the harness job produced the same 2 failed, 39 passed in ~1.26s at both the 5s and 30s ceilings with no abort on either.

The core deliverable is strong. An adversarial pass over cs_invariants.py found no logic error: equivalence with the legacy oracle is exact over all 32×32 transitions and all 720 permutations (zero mismatches), delegation to the per-dimension tables is genuine single-source-of-truth, and the accepted prefixes are exactly the prefix-closure of the 70 valid histories — no dead-end states. The findings below are around the edges of that work, not in it.


Findings

# Phase Severity Description Outcome
phase5-adr-contradicts-sm-09-001-0 spec-conformance ❌ FAIL ADR-0060 claims the vP→VP rule was "covered by no spec"; SM-09-001 is a MUST covering both ephemeral rules — and its "promote before persist" conflicts with CSB-17-003/004 treating them as valid prefix termini, with no relationship link or amendment fix-now
phase8-strenum-identity-comparison-0 code-review ❌ FAIL cs_invariants.py:341 is not on a StrEnumis_valid_cs_history("VFDPXA") raises AttributeError from a bool-returning validator fix-now
phase10-missing-spec-markers-csb17-0 test-coverage ❌ FAIL Zero @pytest.mark.spec markers for four kind: protocol requirements — SR-05-004 (MUST) fix-now
phase9-stale-timeout-numbers-0 notes-currency ❌ FAIL Commit 2 raised the unit tier 5→30s but left 6 references at 5s/20s, incl. conftest.py:58 "12x the unit ceiling" (now 2x) — the arithmetic justifying the value fix-now
phase9-learning-timestamp-not-tz-aware-0 notes-currency ❌ FAIL Both new learning files use timestamp: "2026-08-12"; HistoryEntryFrontmatter rejects it (BW-02-001) — blocks append-history fix-now
phase5-csb17-002-58-count-misattributed-0 spec-conformance ⚠️ IMPROVE CSB-17-002's three conditions yield 72 edges, not the 58 it asserts; 58 needs CSB-17-003 fix-now
phase5-csb17-002-allow-null-unspecified-0 spec-conformance ⚠️ IMPROVE CSB-17-002 forbids the same-state write that CSB-16-001/002 permit; allow_null exists only in code fix-now
phase9-glossary-64-state-stale-0 notes-currency ⚠️ IMPROVE docs/reference/glossary.md:52,213 still say "64-state lattice" — the sweep the learning file calls complete is not fix-now
phase9-flaky-tests-stale-2216-rows-0 notes-currency ⚠️ IMPROVE notes/flaky-tests.md:110-112 cite closed #2216 for the exact three jobs red on this PR; root cause is #2233 fix-now
phase7-agents-md-dangling-reference-0 agents-md ⚠️ IMPROVE AGENTS.md:493See also ISSUE-1784 orphaned onto the wrong pitfall by the new insertion fix-now
phase7-agents-md-import-prohibition-too-broad-0 agents-md ⚠️ IMPROVE Blanket "do not import case_states/" contradicts ADR-0060's reliance on patterns.potential_actions fix-now
phase6-sm-09-001-rationale-points-at-legacy-0 adr-check ⚠️ IMPROVE SM-09-001 still names the demoted legacy module as its enforcement point fix-now
phase6-csb17-001-duplicates-sm-09-002-0 adr-check ⚠️ IMPROVE CSB-17-001 restates SM-09-002 with no relationship link fix-now
phase10-mock-only-precedence-test-0 test-coverage ⚠️ IMPROVE test_explicit_timeout_marker_wins asserts against its own FakeItem stub — cannot fail fix-now
phase10-prefix-converse-untested-0 test-coverage ⚠️ IMPROVE Only one direction of the prefix property is asserted; the converse is what makes the API meaningful fix-now
phase8-dead-trigger-maps-0 code-review ⚠️ IMPROVE CS_EVENT_TO_{VFD,PXA}_TRIGGER unused in vultron/ and the only new names not re-exported fix-now
phase6-cs-invariants-exceeds-500-lines-0 adr-check ⚠️ IMPROVE ADR cites the 500-line CS-18-001 guideline as a driver; the new module is 511 lines fix-now
phase6-adr-numeric-errors-0 adr-check ⚠️ IMPROVE ADR says "92 tests" (93 collected) and "cs.py is already 674 lines" (673) fix-now
phase5-csb17-004-mixed-normative-strength-0 spec-conformance ⚠️ IMPROVE CSB-17-004 is SHOULD but embeds MUST/MUST NOT clauses that inherit SHOULD priority fix-now
phase3-pr-body-omits-agents-md-pitfalls-0 pr-body-format ⚠️ IMPROVE Two new CONCERN-2243 pitfall bullets in AGENTS.md are undisclosed in Changes fix-now
phase9-learning-tags-field-discarded-0 notes-currency ⚠️ IMPROVE tags: has no field in HistoryEntryFrontmatter — silently dropped on archive fix-now
phase11-ci-preexisting-invite-path-422-0 ci-status ⚠️ IMPROVE 3 red jobs, pre-existing, #2233/#2266no code fix in scope; only the notes correction above fix-now

Total: 5 FAIL · 17 IMPROVE · 0 NEW-ISSUE


Clean checks

adr-index --check in sync · spec-lint exit 0 with zero CSB-17 findings · mkdocs build --strict exit 0 · black/flake8/mypy/pyright clean · 106 new tests pass · ADR-0060 matches the template with all required sections and is registered in both index.md and mkdocs.yml · no prior ADR contradicted on the hypercube/CS-enum question · the vultron/case_states/vultron/core/case_states/ path sweep is complete · all 5 modified notes have valid frontmatter · the timeout tiers verified end-to-end (integration 60s, unit 30s, explicit marker wins) · the PR's "two live importers" claim independently confirmed.


Triage artifact: .claude/pr-2275-triage.json
Next step: /pr-execute or /pr-ship to apply fixes.

ahouseholder and others added 2 commits August 13, 2026 00:43
- phase8 — cs_invariants.py used `is not` on a `StrEnum`, so the
  bool-returning predicates raised AttributeError instead of answering;
  added `_as_cs_event` coercion and widened the annotations to accept the
  legacy single-letter strings on every entry point
- phase8 — documented both CS_EVENT_TO_*_TRIGGER maps as intentional public
  API and re-exported them from vultron/core/states/__init__.py
- phase5/phase6 — specs/state-machine.yaml: repointed SM-09-001's rationale
  from the legacy validations module at cs_invariants.py, and scoped it
  explicitly to the persistence boundary
- phase5 — specs/cs-behavior.yaml: CSB-17-001 now `refines` SM-09-002;
  CSB-17-002 attributes the 72-vs-58 transition counts correctly and covers
  the same-state/allow_null carve-out against CSB-16-001/002; CSB-17-003
  `refines` SM-09-001 with the persistence-vs-trajectory reconciliation
- phase5 — split CSB-17-004's embedded MUST clauses into a new MUST
  requirement CSB-17-005 (prefix validation), leaving CSB-17-004 as a
  clean SHOULD
- phase10 — CSB-17-001..005 now carry @pytest.mark.spec markers (SR-05-004)
- phase10 — added the prefix converse: the accepted set is exactly the
  prefix-closure of the 70 valid histories, and no accepted prefix is a
  dead end
- phase10 — added a real-pytest probe class for the integration timeout
  tier, closing the mock-only gap: the marker is added at collection time
  but pytest-timeout reads it in pytest_runtest_protocol
- phase9 — test/conftest.py and test_integration_timeout_tier.py still
  cited the old 5s unit ceiling and its "12x" arithmetic
- phase6/phase9 — ADR-0060 and notes/case-state-model.md claimed the
  `vP -> VP` rule was "covered by no spec"; SM-09-001 is a pre-existing
  MUST covering it. Also corrected 674 -> 673 lines, 92 -> 109 tests, and
  recorded the CS-18-001 exemption for cs_invariants.py
- phase9 — docs/reference/glossary.md called CS a 64-state lattice without
  noting that only 32 states are reachable
- phase9/phase11 — notes/flaky-tests.md: the three red demo/harness rows
  cited closed #2216; repointed at #2233 and #2266 and marked them
  deterministic, not flaky
- phase9 — both learning files carried tz-naive timestamps (BW-02-001) and
  a `tags` field the frontmatter model silently discards; also corrected
  "raised 5s to 20s" to 30s and ISSUE-2237 to ISSUE-2270 as the source
- phase7 — AGENTS.md: relocated the dangling ISSUE-1784 reference to the
  rebase bullet it belongs to, and narrowed the case_states import
  prohibition, which as written also forbade the equivalence-test oracle
  and contradicted the two live importers

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sei-ahouseholder pushed a commit that referenced this pull request Aug 13, 2026
- phase5-ledger-snapshot-core-shaped: the canonical ledger snapshot for a
  partially accepted status was core-shaped (nested rm/vfd, no @context,
  emConsentState or cvdRole), which the fcvcv invariant harness rejects and
  every replica misreads.  Core must not import the wire layer to rebuild the
  object (ADR-0009/0017), so the override is now a field *patch* keyed by wire
  alias, merged onto the snapshot's existing object by
  _merge_snapshot_object_fields.  Shape preservation is structural: the
  override and non-override paths produce identical shapes.  New spec
  RSH-05-009 states the requirement.
- phase8-ratchet-result-discarded: ApplyParticipantStatusFromLedgerNode saved
  the ratcheted status only when the object was absent locally.  Since the
  node appends what it reads *back*, an already-stored object silently
  discarded the ratchet and regressed the replica's RM while the ratchet's own
  warning claimed the opposite.  Saved unconditionally now.
- phase8-absent-case-status-erases-pxa-em: an inbound status with no
  caseStatus asserts nothing about pxa/em; it now carries the receiver's own
  case_status forward instead of blanking both.
- phase8-noop-path-leaves-stale-bb-keys: update() clears both blackboard keys
  unconditionally, before the datalayer guard, so no no-op path inherits a
  previous execution's override (BT-17-003/004).
- phase8-closed-to-closed-mislabeled-refused: addressed in part.  The
  operator-facing WARNING now distinguishes "rewrote dimension(s) X" from
  "blocked dimension(s) X with no change to the asserted value".  The
  reviewer's proposed reorder of _rm_is_acceptable was NOT applied: making
  CLOSED->CLOSED acceptable empties `refused`, which makes
  ValidateRMTransitionNode re-reject the transition (reintroducing #2235) and
  lets a pure no-op status be appended and ledger-committed.
- phase10-no-snapshot-schema-assertion: assert the committed snapshot keeps
  the sender's wire shape (rmState/vfdState/emConsentState/cvdRole/@context,
  nested caseStatus, no core dimension dicts, no stale snake_case twins), plus
  omitted-caseStatus and already-stored-object coverage.
- phase10-no-blackboard-leak-test: back-to-back executions, the commit node's
  ID match in both directions, resolve_dimension_filter's mismatch branch, and
  the datalayer-missing clear.
- phase10-monotonic-helpers-untested: exhaustive table-driven coverage of
  is_monotonic_vfd_forward / is_monotonic_pxa_forward over every ordered pair,
  with a bitmask subset oracle independent of the implementation.
- phase6-adr-number-collision: renumbered ADR-0060 -> ADR-0061; open PR #2275
  (older) keeps 0060.  Note that PR #2210 claims 0059, which is already taken
  on main and will need its own renumber.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The base sync brought in #2277's learning: a count restated next to a
requirement reference drifts silently, because the spec linter validates
that the reference resolves, not the prose around it. CSB-17-002 now
restates both 72 (conditions 1-3 alone) and 58 (all four), and only 58 was
asserted anywhere.

Adds `test_structural_conditions_alone_admit_72_transitions`, which derives
the conditions-1-3 set from the per-dimension tables and asserts the
72 -> 58 reduction plus the fact that every removed edge leaves an
ephemeral state — so the attribution CSB-17-002 makes is now testable
rather than prose.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`origin/main` 06bf60c fails 15 demo/harness jobs on its own, so the six
that fail here are base-owned. #2266 promoted `engage_case` to a universal
invariant asserted in all nine scenarios, which turned the invite-path 422
of #2233 from one silent gap into a red Invariant Harness job per scenario.

Repoints `fcvcv`/`fcv-reject`/`fv` Invariant Harness at #2233 (they cited
closed #2121 or nothing), adds the `fv` row, and states plainly that these
are deterministic and must not be "fixed" on a feature branch.

Evidence recorded on the issue:
#2233 (comment)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@sei-ahouseholder sei-ahouseholder left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Duplicate post — see the pr-execute report below.)

@sei-ahouseholder sei-ahouseholder left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 pr-execute — PR #2275

22 findings from triage: 21 fixed, 1 skipped (base-owned CI), 0 deferred.

Commits

Ref What
639f6192 21 triage findings, one batch
adf6272a merge origin/main 06bf60c2 — clean, no conflicts
2506edbc pin CSB-17-002's restated 72-transition count
1ad9b54a record the six CI jobs blocked by #2233

FAIL findings (5/5 fixed)

  • cs_invariants.py used is not on a StrEnum — the bool-returning
    predicates raised AttributeError instead of answering, so any caller
    migrating from the legacy string API got an exception rather than a verdict.
    Added _as_cs_event coercion and widened the annotations to CSEvent | str
    on every entry point. Re-verified equivalence against the legacy oracle:
    0 mismatches over 720 permutations × both input forms, 70 valid.
  • ADR-0060 contradicted SM-09-001 — it claimed the vP → VP rule was
    "covered by no spec", but SM-09-001 is a pre-existing MUST covering both
    ephemeral rules. Corrected in the ADR and notes/case-state-model.md, and
    reconciled properly: SM-09-001 binds at the persistence boundary,
    CSB-17-003/005 at the trajectory boundary. CSB-17-003 now refines
    SM-09-001 with that note; SM-09-001's rationale says so too.
  • CSB-17 kind: protocol requirements had no @pytest.mark.spec markers
    (SR-05-004) — CSB-17-001…005 are now all marked.
  • Stale timeout numberstest/conftest.py and the tier test still cited
    the old 5s unit ceiling, including the "12x the unit ceiling" arithmetic that
    justifies 60s (now 2x).
  • Learning frontmatter timestamps were tz-naive (BW-02-001) — the read model
    rejects those. Both files fixed.

Notable IMPROVE fixes (16/17 fixed)

  • Split CSB-17-004's embedded MUST clauses into a new MUST, CSB-17-005
    (prefix validation), leaving CSB-17-004 a clean SHOULD. CSB-17-005 pins the
    prefix rule in both directions — the accepted set is exactly the
    prefix-closure of the 70 valid histories — with new tests for the converse.
  • Closed the mock-only gap in the timeout tests. The existing tests
    exercised the hook against a stub, which cannot show that pytest-timeout
    honours a marker added at collection time when it reads it in
    pytest_runtest_protocol. Added a real nested pytest session that reports the
    timeout each item actually resolved to (30 / 60 / explicit-7).
  • tags: in the learning frontmatter is silently discarded — no such field
    on the model, and only these two files used it. Folded into the body.
  • Glossary called CS a 64-state lattice without noting only 32 are reachable.
  • AGENTS.md's case_states import prohibition was over-broad — as written it
    forbade the equivalence-test oracle and contradicted the two live importers.
  • A dangling ISSUE-1784 reference was relocated to the rebase bullet it
    actually belongs to.

One finding was resolved by declaration rather than deletion:
CSB-17-001 duplicates SM-09-002 → CSB-17-001 now refines SM-09-002. It is
kept because it is the testable enum-level ratchet; SM-09-002 is the
protocol-level MUST_NOT.

Found during the base sync

The merge brought in #2277's learning: a count restated next to a requirement
reference drifts silently, because the linter validates that the reference
resolves, not the prose around it.
My own CSB-17-002 fix had just introduced
one — the 72 in "these three conditions alone admit 72 transitions". 58 was
asserted somewhere; 72 was not. Added
test_structural_conditions_alone_admit_72_transitions, which derives the
conditions-1–3 set from the per-dimension tables and asserts the 72 → 58
reduction plus the fact that every removed edge leaves an ephemeral state.

⏭ Skipped: CI (base-owned)

phase11-ci-preexisting-invite-path-422-0#2233

Six jobs are red. None are branch-owned: origin/main at the exact merged
base 06bf60c2 fails 15 of these jobs on its own

(run 31655077093),
a strict superset of this PR's 6.

Root cause is the invite-path engage-case HTTP 422 of #2233. #2266 then
promoted engage_case to a universal invariant asserted in all nine scenarios,
which converted one silent gap into a red Invariant Harness job per
scenario
. Evidence recorded on
#2233,
and notes/flaky-tests.md repointed — those rows previously cited the closed
#2216 and #2121 — with an explicit note that they are deterministic, not flaky.

No code fix attempted, per the triage artifact.

Tests

Suite Result
Unit 6714 passed, 370 skipped, 1104 deselected, 2 xfailed, 5552 subtests — 2 consecutive clean runs, 116s
Integration 1101 passed, 2 xfailed, 1 xpassed, exit 0, 72s
xfail ratchet all 6 referenced issues verified open (#1991, #1992, #1898, #2150, #1993, #1994)
spec-lint exit 0, 62 warnings — the pre-existing baseline, no CSB-17 findings
adr-index --check in sync
mkdocs build --strict exit 0

⚠️ One honest caveat about this PR's own fix. Two earlier full-suite runs
aborted at the new 30s unit ceiling, with a 1-minute load average near 30 on
20 cores
— a background graphify rebuild launched by the commit hook. The
same suite then passed twice at 116s on an idle machine. The tier values are
sized against honest test duration, not against an oversubscribed host, so 30s
is not proof against arbitrary CPU starvation. This is now stated in the PR body
rather than left as an implicit overclaim.

Merge state

MERGEABLE / CLEAN — base main, merged 06bf60c2 cleanly (sync exit 0, no
conflicts).

Other

  • Review threads: 0 unresolved — nothing to reply to.
  • New issues filed: none (no NEW-ISSUE findings in triage).
  • PR body updated: corrected test counts (93 → 110, 9 → 16), CSB-17 spec count
    (4 → 5), disclosed the AGENTS.md changes, and refreshed Verification.

Artifact: .claude/pr-2275-execute.json

@sei-ahouseholder sei-ahouseholder left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ pr-verify — PR #2275

Overall verdict: GAPS-FOUND

Merge state: MERGEABLE (UNSTABLE) — base main, not draft, no conflict
markers at HEAD.
CI: 15 pass / 6 fail at HEAD 1ad9b54a.
Findings: 22/22 accounted for — 21 CONFIRMED at HEAD, 1 NOTED.

The blocking flag is UNVERIFIED-CI-FAILING, and nothing else. Every finding
execute claimed to fix is present at HEAD; the branch merges cleanly; the base
sync is verified. But six CI jobs are red, and a PR does not become ready to
merge because its red jobs belong to someone else.

Why the verdict is not READY-TO-MERGE

The six failures are provably not branch-owned:

origin/main at 06bf60c2 — the exact commit this branch merged — fails 15
of these jobs on its own
(run 31655077093).
This PR's 6 are a strict subset. Signatures at HEAD are unchanged from the base:

Job class Signature Cause
Invariant Harness ×4 AssertionError: Expected eventType 'engage_case' not found in case-actor log. #2266 promoted engage_case to a universal invariant asserted in all nine scenarios; #2233 means it is never emitted
Demo Integration ×2 AssertionError: Timed out waiting for actor ... rm_state to be in {...} #2233 — participant RM state written to the standalone record but read from the inline case copy, so the participant never reaches ACCEPTED

So the honest reading is: this branch is clean and cannot merge green, because
main is not green.
Fixing that is #2233's job, not this PR's — execute
correctly declined to attempt a code fix and instead recorded the evidence on
#2233
and repointed notes/flaky-tests.md (those rows had been citing the closed
#2216 and #2121).

FAIL findings — 5/5 CONFIRMED at HEAD

Finding Check at HEAD Verdict
phase8-strenum-identity-comparison-0 cs_invariants.py:381 reads != event; _as_cs_event present; no residual is not compare CONFIRMED
phase5-adr-contradicts-sm-09-001-0 no "covered by no spec" / "enforced nowhere" text remains in ADR-0060 or notes/case-state-model.md; specs/cs-behavior.yaml:1581 carries the SM-09-001 relationship CONFIRMED
phase10-missing-spec-markers-csb17-0 CSB-17-001…005 all marked (1/2/1/1/4 markers) CONFIRMED
phase9-stale-timeout-numbers-0 no stale current-state 5s claims; timeout = 30 cited in both files. The one remaining "5s ceiling" mention is past tense — "they were tripping a 5s ceiling" — and correct as history CONFIRMED
phase9-learning-timestamp-not-tz-aware-0 verified against the model, not just the text: both files parse through HistoryEntryFrontmatter to 2026-08-12T00:00:00+00:00, tz=UTC CONFIRMED

IMPROVE findings — 16/16 CONFIRMED at HEAD

csb17-002-58-count-misattributed · csb17-002-allow-null-unspecified ·
glossary-64-state-stale · flaky-tests-stale-2216-rows ·
agents-md-dangling-reference · agents-md-import-prohibition-too-broad ·
sm-09-001-rationale-points-at-legacy · csb17-001-duplicates-sm-09-002 ·
mock-only-precedence-test · prefix-converse-untested ·
dead-trigger-maps · cs-invariants-exceeds-500-lines ·
adr-numeric-errors · csb17-004-mixed-normative-strength ·
learning-tags-field-discarded · pr-body-omits-agents-md-pitfalls

NOTED — 1

phase11-ci-preexisting-invite-path-422-0#2233, confirmed real and
OPEN. Correctly skipped rather than fixed.

Local test evidence (from execute)

Suite Result
Unit 6714 passed, 370 skipped, 1104 deselected, 2 xfailed, 5552 subtests — 2 consecutive clean runs
Integration 1101 passed, 2 xfailed, 1 xpassed, exit 0
xfail ratchet all 6 referenced issues verified OPEN
spec-lint / adr-index --check / mkdocs --strict exit 0, 0, 0

CI's Tests (pytest), Lint (black/flake8/mypy/pyright), Spec Lint, CodeQL
and docs-build-check all pass at HEAD, so the branch's own quality gates
are green. Only the demo/harness jobs — which are red on main too — are not.

Note on artifact schema

Execute's merge_state block recorded sync_exit_code: 0 rather than the
synced: true key this skill looks for. I did not treat that as
UNSYNCED-EXECUTE: the substantive condition is independently satisfied and was
re-checked live here — sync exit 0, conflicts: [], merged base 06bf60c2
recorded, live MERGEABLE, and no conflict markers at HEAD. Flagging it would
have reported a defect that does not exist. Recording the key-name mismatch here
so the discrepancy is visible rather than silently resolved in the PR's favour.

What to do next

Nothing on this branch. It is finding-complete and mergeable; it is waiting on
#2233. Two options for a human:

  1. Merge anyway, accepting the six known-red base-owned jobs (they are red on
    main regardless of this PR).
  2. Land #2233 first, then re-run /pr-verify 2275 — no execute re-run is
    needed, since the gap is CI status rather than an unresolved finding.

Artifacts retained (GAPS-FOUND does not clean up): .claude/pr-2275-triage.json,
.claude/pr-2275-execute.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 301+ diff lines or 7+ ACs

Projects

None yet

2 participants