Repository navigation
Qualify actual retention through Catalog field computation offline - #130
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Pin and run the integration harness .github/workflows/stream-binding-proposal.yml, proposals/retained-field-integration-2207/test_integration.py, proposals/retained-field-integration-2207/build.mjs |
The test setup verifies a pinned API checkout and builds the evaluator. Helpers exercise locked reads and evaluation. The workflow installs dependencies and runs the suite for PostgreSQL 15 and 17. |
Check field results and currentness proposals/retained-field-integration-2207/test_integration.py |
Tests check complete and unknown field results, retries and revisions, refusals after source changes, and recomputation after a supported correction. |
Check budgets and transaction locks proposals/retained-field-integration-2207/test_integration.py, proposals/retained-field-integration-2207/PROTOCOL.md |
Tests check UTF-8 and storage budgets, plus lock contention during evaluation. The protocol documents the test flow, limits, and stated production and UI handoff gates. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Other
Possibly related issues
recoupable/app#2305— The PR adds end-to-end conformance tests for retained writes, Catalog evaluation, source changes, and currentness.recoupable/app#2277— The tests cover source-bound field computation, provenance, revisions, and corrections.recoupable/app#2278— The tests check binding, query, provenance, and refusal states in Catalog field results.recoupable/app#2116— The suite exercises versioned evidence reads and behavior after correction, withdrawal, and revocation.
Possibly related PRs
- recoupable/database#127 — This PR tests the
read_bindingbehavior defined by PR#127as part of an end-to-end flow through the Catalog evaluator.
Suggested reviewers: sweetmantech
Merge Risk: 
ed9e8
-
The qualification suite is ready to merge after normal CI checks; it does not activate production behavior.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "Qualify actual retention through Catalog field computation offline" matches the main change. The PR adds an offline integration test suite. The suite runs the actual retention writer and th… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "Qualify actual retention through Catalog field computation offline" matches the main change. The PR adds an offline integration test suite. The suite runs the actual retention writer and the pinned Catalog evaluator against a disposable PostgreSQL database. The title states the offline scope. It does not mention the workflow change or the documentation, which is acceptable because those support the main change. The title is specific, concise, and uses no vague terms.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 14 functions across 2 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
The Catalog evaluator tests supply qualified/unavailable boundary fixtures without executing retention and the authority read. This adds a disposable integration harness that calls the actual writer, obtains the exact
read_bindingenvelope from its receipt, and invokes the pinned API #1190 evaluator while the read transaction remains open.Eight integration tests cover all four fields and complete binding/provenance, missing versus observed zero, retry versus current document revision, supported correction with new retention/recomputation, withdrawal and membership revocation, competing mutation locks, UTF-8 refusal and source-history budget denial. CI keeps the existing 45 reader/writer tests and adds the integration suite on PostgreSQL 15/17 with API head
6ba799dbb67be284adcd6f8bcd8fcde2faa6c2b8, verified evaluator/recipe/helper hashes and frozen dependencies.Validation at
ed9e805369f1a9ca52174928b5b3b9c24d9508be: CI matrix passed all 45 existing tests plus all eight integration tests on both PostgreSQL 15 and 17, including cleanup. All other Actions checks passed; zero review threads. Local eight-test runs passed on PostgreSQL 15/17. Independent source review passed, including an independent PostgreSQL 15 run and final assertion-only delta review.Offline qualification only: no migration, grant, runtime adapter, provider ingestion, UI descriptor projection or production activation. Currentness is protected only during the held transaction; later publication/caching needs its own protocol. Root owns merge and activation gates. See the protocol for reproducible commands and bounded claims.
Summary by CodeRabbit
Tests
Documentation