Implementation Plan: T5-P3-A3-WP3 Produce Merge-Blocking CI Tests for Codex Deterministic Conformance#4149
Conversation
Seal the CodexEventType/CodexItemType vocabulary, hook event formats, and config.toml schema template against undetected drift. All tests are pure Python assertions against committed JSON fixtures — no live CLI, no subprocess, no network I/O. - Add --update-fixtures to pytest_addoption in tests/conftest.py - Add update_fixtures fixture in tests/execution/backends/conftest.py - Create test_codex_deterministic_conformance.py with 4 test classes: - TestCodexEventTypeVocabulary (parametrized + sealed meta-test) - TestCodexItemTypeVocabulary (parametrized + sealed meta-test) - TestCodexHookEventFormatFixture (hash + structural comparison) - TestCodexConfigTomlSchemaTemplate (key coverage + constant pinning) - Register new test file in tests/execution/AGENTS.md backends/ table Run pytest --update-fixtures -n0 to regenerate fixtures after a deliberate vocabulary or format change.
…ce tests The test constructs fixture filenames using member.value (e.g. "thread.started") producing event_thread.started.json, but existing fixtures used underscores (event_thread_started.json). Create dotted-name copies so all 9 non-UNKNOWN CodexEventType members resolve their expected fixture paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: approved_with_comments
| "tests/execution/backends/test_codex_deterministic_conformance.py" | ||
| ) | ||
|
|
||
| def test_update_fixtures_writes_snapshot( |
There was a problem hiding this comment.
[warning] tests: test_update_fixtures_writes_snapshot skips in normal CI (--update-fixtures not set), so it contributes zero assertions on every standard run. The post-write assertions (L190-L191) only verify json.dumps/json.loads round-trip fidelity — they cannot fail independently of the write. Either exclude via a dedicated mark filtered in CI, or remove the round-trip assertions as they test stdlib behavior.
There was a problem hiding this comment.
Valid observation — flagged for design decision. The post-write assertions in test_update_fixtures_writes_snapshot verify only write-integrity (file is written, parseable, and round-trips the in-memory values) — they cannot detect a logic error in what was computed before writing. Drift detection is covered by test_hook_snapshot_matches_live_registry which runs unconditionally. Whether the write-integrity check merits dedicated CI exclusion or assertion removal is a design question for human review.
|
|
||
|
|
||
| @pytest.fixture | ||
| def update_fixtures(request: pytest.FixtureRequest) -> bool: |
There was a problem hiding this comment.
[warning] cohesion: The update_fixtures fixture is defined only in tests/execution/backends/conftest.py but --update-fixtures CLI option is registered in the top-level tests/conftest.py. If future tests outside tests/execution/backends/ need this fixture, it will be unavailable. Consider co-locating the fixture at the top-level conftest.py alongside the option registration.
There was a problem hiding this comment.
Valid observation — flagged for design decision. The --update-fixtures CLI option is registered in the top-level tests/conftest.py per pytest requirements, while the update_fixtures fixture wrapping it is defined only in tests/execution/backends/conftest.py — intentional co-location with its sole consumer. Moving the fixture to the top-level conftest is a valid future-proofing step if the option is expected to become reusable, but requires a human decision on whether this is expected to remain backends-only.
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.
…template regen test Post-write check in test_update_fixtures_regenerates_template verified only tool_output_token_limit but omitted model_auto_compact_token_limit, creating an asymmetric check against the two-constant template it just wrote. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
deleted the
t5-p3-a3-wp3-produce-merge-blocking-ci-tests-no-live-cli-req/4011-2
branch
1 participant
Summary
Add
tests/execution/backends/test_codex_deterministic_conformance.pywith four test classes that seal theCodexEventType/CodexItemTypevocabulary, hook event formats, and config.toml schema template against undetected drift. Register--update-fixturesin the existingpytest_addoption()intests/conftest.pyand expose it as a pytest fixture. All tests are pure Python assertions against committed JSON fixtures — no live CLI, no subprocess, no network I/O.Closes #4011
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260628-181308-999735/.autoskillit/temp/make-plan/t5_p3_a3_wp3_deterministic_conformance_plan_2026-06-28_181500.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
* Step used a non-Anthropic provider; caching behavior may differ.
Token Efficiency
Model Usage Breakdown