refactor: unify assertion and run status vocabularies - #3657
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThe PR centralizes ChangesOperation status lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant embed_command
participant _record_archive_backfill_run
participant ops_write
participant OPS_database
embed_command->>_record_archive_backfill_run: pass CLI backfill status
_record_archive_backfill_run->>_record_archive_backfill_run: map complete or stopped to OperationStatus
_record_archive_backfill_run->>ops_write: persist normalized status
ops_write->>OPS_database: write embedding catchup run
OPS_database-->>_record_archive_backfill_run: store completed or interrupted status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
71e995e to
b88deb1
Compare
![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 `@polylogue/storage/sqlite/archive_tiers/ops.py`:
- Around line 7-12: Update the OPS schema convergence logic around
OPS_SCHEMA_VERSION and _OPS_RUN_STATUS_CHECK to repair existing ops.db files,
not only newly created tables. Add idempotent helpers that drop and recreate the
status CHECK constraints for ingest_attempts and embedding_catchup_runs using
OPERATION_LIFECYCLE_STATUSES, ensuring interrupted is accepted, and invoke them
during bootstrap after OPS_DDL.
🪄 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 Plus
Run ID: e79e3766-8d95-4b58-b4d2-80676efe29b2
📒 Files selected for processing (13)
polylogue/api/archive.pypolylogue/cli/commands/embed.pypolylogue/cli/commands/note.pypolylogue/core/enums.pypolylogue/maintenance/planner.pypolylogue/operations/operation_status.pypolylogue/readiness/capability.pypolylogue/storage/sqlite/archive_tiers/ops.pypolylogue/storage/sqlite/archive_tiers/ops_write.pytests/unit/cli/test_embed_status_fast.pytests/unit/cli/test_note.pytests/unit/core/test_readiness_capability.pytests/unit/operations/test_operation_contract.py
Problem: CAVEAT was accepted by the assertion candidate allowlists and readers but omitted from the capture-kind map, so terminal and agent capture could not produce it.\n\nWhat changed: add CAVEAT to the shared capture map and the existing terminal note choices. Preserve RUN_STATE after focused tests proved the transform-candidate producer and promotion reader still use its historical value.\n\nCompatibility/migration: no stored vocabulary changed. Existing HIGHLIGHT, PROMPT_EVAL, BLOCKER, HANDOFF, LESSON, and RUN_STATE semantics remain intact.\n\nRef polylogue-jwqj
Problem: ingest attempts, embedding catch-up, maintenance planning, and operation acknowledgements carried duplicated lifecycle vocabularies, including interrupted versus cancelled for the same non-error stop. The embed writer translated public values with an untyped fallback.\n\nWhat changed: make OperationStatus the canonical low-level enum, add interrupted, generate both ops-tier lifecycle checks from one subset, type status boundaries, map the public complete/stopped payload explicitly, and replace the planner-local enum with a compatibility alias to the canonical type. Interrupted operations now project to blocked readiness.\n\nCompatibility/migration: public complete/stopped payload values remain unchanged. The ops tier is disposable, so its bootstrap DDL uses interrupted consistently; the legacy monolith reader still interprets historical rows unchanged. Existing imports of BackfillStatus resolve to the canonical OperationStatus.\n\nRef polylogue-oj4oo
Problem: same-version OPS bootstrap replays CREATE TABLE IF NOT EXISTS, which leaves an existing embedding_catchup_runs CHECK admitting cancelled but rejecting the new interrupted lifecycle status. What changed: reuse the canonical OPS table DDL in a row-preserving status-check convergence helper. Rebuild only stale ingest_attempts or embedding_catchup_runs tables, normalize legacy cancelled embedding rows to interrupted, and recreate the ingest indexes. Bootstrap invokes the helper after additive column convergence. Compatibility/migration: OPS remains disposable and schema version 1. Existing rows are copied transactionally; cancelled embedding rows retain their non-error stop meaning as interrupted. Verification: devtools test tests/unit/storage/test_archive_tiers_ops_write.py tests/unit/storage/test_schema_drift_samples.py (25 passed). devtools verify --quick passed all checks except the inherited stale layering baseline entry for polylogue/mcp/server_prompts.py, unchanged from origin/master. Co-Authored-By: Claude <noreply@anthropic.com>
b88deb1 to
aa7a1b8
Compare
|
Verified against the OPS bootstrap and lifecycle paths: same-version opens re-run Added idempotent OPS writer convergence invoked after additive column setup. It rebuilds only stale Verification: |
1 participant
Summary
This PR wires the missing CAVEAT assertion capture route and consolidates run-lifecycle status handling across operation surfaces, maintenance planning, and disposable ops ledgers. It preserves public capture and embedding display values while making the internal lifecycle boundary typed.
Problem
The assertion audit found CAVEAT present in candidate readers and allowlists but absent from the production capture map. The same audit initially suggested RUN_STATE was dead, but focused production tests proved the transform-candidate pipeline still emits and promotes that historical value. Run and attempt lifecycle states were independently declared in ops DDL, maintenance planning, and operation acknowledgements, with interrupted and cancelled representing the same non-error stop in adjacent subsystems.
Solution
BackfillStatusimport as an alias while making the planner model use canonicalOperationStatus.OperationStatusintocore.enums, addinterrupted, generate both ops-tier lifecycle checks from one subset, and re-export the existing operations import path.completeandstoppedvalues tocompletedandinterrupted. Map interrupted operations to blocked readiness and preserve the legacy monolith reader's historical statuses.Acceptance matrix
note --kind caveatcapture test writes a candidate returned by the candidate-review readerOperationStatus, one lifecycle subset, and both ops DDL checks are exercised by real writer/readback testscompleteandstoppedremain payload values;stoppedpersists asinterruptedthrough_record_archive_backfill_runVerification
python -m devtools test tests/unit/cli/test_note.py tests/unit/storage/test_archive_tiers_assertions.py tests/unit/operations/test_operation_contract.py tests/unit/core/test_readiness_capability.py tests/unit/cli/test_embed_status_fast.py tests/unit/sources/test_live_watcher_catchup_order.py tests/unit/daemon/test_live_ingest_attempt_progress.py tests/unit/maintenance/test_planner_contract.py-> 196 passed.python -m devtools verify --quick-> exit 0; format, lint, mypy, render, layering, policy, and schema promotion steps passed.python -m devtools render openapiandpython -m devtools render cli-output-schemas-> synchronized; no generated file diff was required.python -m devtools render all --check-> all checked surfaces synchronized.python -m devtools verify --seed-testmon --skip-slowreached the seeded broad suite and exposed unrelated baseline failures intests/unit/daemon/test_daemon_bulk_rebuild_responsiveness.py::test_small_writer_actors_stay_responsive_during_bulk_rebuild_drainand bothtests/unit/sources/test_live_watcher_locking.py::test_real_watcher_writer_routes_cannot_pin_process_exitcases. The run was stopped after these failures were established; the affected focused suite remained green.Ref polylogue-jwqj
Ref polylogue-oj4oo
Summary by CodeRabbit
New Features
caveatandprompt_evalnote types.interruptedoperation status for stopped or halted processes.Bug Fixes
Tests