test(storage): cover assertion overlay parity (#1883) - #2078
Conversation
Problem: #1883 still needed public-surface evidence for assertion-backed overlay parity and for the explicit tag/metadata table-backed exception. Helper-level tests covered much of the substrate, but they did not prove the Polylogue facade paths preserve assertion metadata. What changed: add user-state contract tests for public blackboard writes carrying author/evidence/staleness/context policy into NOTE assertions, and for tag/metadata mutations remaining table-backed user metadata rather than lifecycle assertions. Verification: - nix develop --command devtools test tests/unit/storage/test_user_state_contracts.py - nix develop --command devtools verify --quick Ref #1883
Problem: default devtools verify can terminate a long pytest-testmon run before pytest-json-report or JUnit flushes, leaving report_path null and no node-level evidence for failures observed before timeout. What changed: load a small pytest plugin during verify pytest lanes that writes per-test call outcomes to .cache/verify/current-pytest-events.jsonl, persist captured pytest output to .cache/verify/current-pytest-output.log, expose both paths in verify metadata, and clear stale artifacts before each pytest step. Verification: - nix develop --command devtools test tests/unit/devtools/test_verify.py tests/unit/storage/test_user_state_contracts.py - nix develop --command devtools verify --quick Ref #2006
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c0a2e945e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if getattr(report, "when", None) != "call": | ||
| return |
There was a problem hiding this comment.
When a selected test errors before its call phase, such as a fixture/setup failure, pytest emits a setup report and never emits a call report, so this early return leaves .cache/verify/current-pytest-events.jsonl with no node-level evidence for that failure. That undercuts the new timeout/stall post-mortem path precisely for common verify failures that happen outside the test body; include failed/error reports from setup/teardown (and possibly collection) instead of filtering all non-call reports out.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@devtools/pytest_progress_plugin.py`:
- Around line 37-39: The file write operations for the event ledger
(path.parent.mkdir and path.open with handle.write) can raise OSError which will
cause the pytest hook to fail entirely. Wrap these write operations in a
try-except block that catches OSError and handles it gracefully by logging the
error without re-raising the exception, ensuring that telemetry failures do not
cause the pytest run to fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a0e07f46-315a-4ba8-b2d8-6617fc6f016b
📒 Files selected for processing (4)
devtools/pytest_progress_plugin.pydevtools/verify.pytests/unit/devtools/test_verify.pytests/unit/storage/test_user_state_contracts.py
Problem: review found that the new verify event ledger missed setup/teardown failures and could let optional telemetry write failures escape from the pytest hook. What changed: record failed setup/teardown reports in addition to normal call reports, make event-file writes best-effort on OSError, and add direct plugin tests for setup failure capture and write-failure containment. Verification: - nix develop --command devtools test tests/unit/devtools/test_verify.py tests/unit/devtools/test_pytest_progress_plugin.py - nix develop --command devtools verify --quick Ref #2006
1 participant
Summary
Adds public-surface assertion parity coverage for #1883 and fixes the verification harness gap that surfaced while running the default baseline. The storage tests now prove blackboard writes through
Polylogue.post_blackboard_note()preserve assertion metadata, and they prove ordinary session tags/session metadata remain the intentional table-backed user metadata exception rather than lifecycle assertions.Problem
#1883 had helper-level assertion write-through coverage and readiness audit coverage, but the durable public-facade proof was incomplete for two important cases: blackboard assertion metadata and the explicit tags/metadata exception. While validating the branch,
devtools verifyselected a very large pytest-testmon set, ran for 45 minutes, then timed out at 82%. Because pytest-json-report never flushed, verify reportedreport_path: nulland lost node-level failure evidence despite many visible failures. That made the broad gate hard to act on.Solution
tests/unit/storage/test_user_state_contracts.pywith public-facade assertions for blackboard author/evidence/staleness/context-policy metadata.Polylogue.add_tag()andPolylogue.set_metadata(), matching Unify user overlays as evidence-linked assertions #1883's current design: simple tag/metadata rows are user metadata, not assertion lifecycle claims.devtools.pytest_progress_plugin, loaded by everydevtools verifypytest lane, which writes per-test call outcomes incrementally to.cache/verify/current-pytest-events.jsonl..cache/verify/current-pytest-output.logand expose both paths in verify metadata so timeout/stall terminations leave usable evidence.Verification
nix develop --command devtools test tests/unit/storage/test_user_state_contracts.py->9 passed.nix develop --command devtools test tests/unit/devtools/test_verify.py tests/unit/storage/test_user_state_contracts.py->52 passed.nix develop --command devtools verify --quick->exit_code: 0at8c0a2e94.nix develop --command devtools verifybefore the harness fix: static gates passed, pytest-testmon timed out after 2700s at 82% withreport_path: null. This PR fixes that evidence-loss path; I did not rerun another 45-minute default baseline after the harness patch.Ref #1883
Ref #2006
Summary by CodeRabbit
Tests
Chores