feat: add provider-free P2-P6 evaluation - #298
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds provider-free P2–P6 phase schemas and evaluators, exposes them through a bounded CLI command, packages integrity metadata, and adds unit, contract, acceptance, documentation, and inventory coverage. Results remain advisory and preserve baseline fallback. ChangesProvider-free phase evaluation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant ReceiptCLI
participant PhaseEvaluator
participant BaselineFallback
Operator->>ReceiptCLI: provide bounded canonical JSON
ReceiptCLI->>PhaseEvaluator: dispatch p2 through p6 record
PhaseEvaluator->>BaselineFallback: preserve unchanged baseline on blocked evidence
PhaseEvaluator-->>ReceiptCLI: return structured advisory result
ReceiptCLI-->>Operator: emit canonical JSON response
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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
context-guard-kit/phase_evaluation.py (1)
710-717: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
confidence_basis_pointsomits malformed trials.
confidences.append(confidence)runs only inside the_exact_dict(trial, _P4_TRIAL)branch. A trial with unknown or missing keys contributes an entry totrials(line 774) but no entry toconfidences. The two output arrays then have different lengths, andevaluated_trial_countmatches neither.Append the default
0for malformed trials so the arrays stay index-aligned.🤖 Prompt for 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. In `@context-guard-kit/phase_evaluation.py` around lines 710 - 717, Ensure the trial-evaluation logic around _exact_dict and confidence_basis_points appends a confidence value for every trial, including malformed records. Initialize malformed trials’ confidence to 0 before appending, while preserving validation and low-confidence checks for valid values so trials, confidences, and evaluated_trial_count remain index-aligned.
🤖 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 `@packages/context-guard-receipt/python/context_guard_receipt/cli.py`:
- Around line 1455-1473: Restrict phase_id validation in the evaluator lookup
within the CLI evaluation flow to string values before calling the mapping’s get
method. Ensure objects, arrays, and other non-string JSON values return
evaluation_phase_rejected with exit 65, while valid string phase IDs retain the
existing evaluator dispatch.
- Around line 1474-1479: Update the evaluation result encoding near evaluator
and write_stdout to pass an explicit output limits object sized for the schema’s
maximum valid result, rather than relying on canonical_json_bytes defaults.
Import and catch CanonicalJSONError separately, returning the distinct
evaluation_result_rejected error with the appropriate non-internal failure
status; narrow the existing broad Exception handler to unexpected failures, and
extend the packaged CLI contract test if that error code is added.
In `@packages/context-guard-receipt/schemas/phase-evaluation-p4.schema.json`:
- Around line 102-110: Align all three validation sites with the result schema:
in packages/context-guard-receipt/schemas/phase-evaluation-p4.schema.json lines
102-110, set bypass_reasons item maxLength to 128 and use
^[a-z][a-z0-9_]{0,127}$; in context-guard-kit/phase_evaluation.py lines 719-726
and
packages/context-guard-receipt/python/context_guard_receipt/phase_evaluation.py
lines 719-726, replace _valid_identifier validation with an identical matcher
for that result pattern, keeping both Python copies byte-identical.
---
Nitpick comments:
In `@context-guard-kit/phase_evaluation.py`:
- Around line 710-717: Ensure the trial-evaluation logic around _exact_dict and
confidence_basis_points appends a confidence value for every trial, including
malformed records. Initialize malformed trials’ confidence to 0 before
appending, while preserving validation and low-confidence checks for valid
values so trials, confidences, and evaluated_trial_count remain index-aligned.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: edc14a42-45a4-41c1-9ff2-9cbba0d69ea0
📒 Files selected for processing (23)
CHANGELOG.mdcontext-guard-kit/bash_reference_policy.pycontext-guard-kit/phase_evaluation.pypackages/context-guard-receipt/README.mdpackages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/dev/package_check.pypackages/context-guard-receipt/dev/packaged_acceptance.pypackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/python/context_guard_receipt/cli.pypackages/context-guard-receipt/python/context_guard_receipt/phase_evaluation.pypackages/context-guard-receipt/schemas/phase-evaluation-p2.schema.jsonpackages/context-guard-receipt/schemas/phase-evaluation-p3.schema.jsonpackages/context-guard-receipt/schemas/phase-evaluation-p4.schema.jsonpackages/context-guard-receipt/schemas/phase-evaluation-p5.schema.jsonpackages/context-guard-receipt/schemas/phase-evaluation-p6.schema.jsonpackages/context-guard-receipt/schemas/phase-evaluation-result.schema.jsonpackages/context-guard-receipt/tests/contract/test_g001_distribution_contract.pypackages/context-guard-receipt/tests/contract/test_g015_phase_evaluation_cli.pyplugins/context-guard/bin/bash_reference_policy.pyresearch/p2-p6-provider-free-implementation.mdresearch/token-savings-roadmap.mdtests/test_contextguard_stage2_feasibility.pytests/test_phase_evaluation.py
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 `@tests/test_phase_evaluation.py`:
- Around line 572-584: The malformed-identity test does not ensure any track
reports exist before validating their blockers. In
test_malformed_phase_identity_cannot_report_track_readiness, assert that
result["tracks"] contains at least one report before the all(...) check, keeping
the existing malformed_record assertions unchanged.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6faf8d99-e4cc-4b20-a294-50d68aa743c9
📒 Files selected for processing (8)
context-guard-kit/bash_reference_policy.pycontext-guard-kit/phase_evaluation.pypackages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/python/context_guard_receipt/phase_evaluation.pyplugins/context-guard/bin/bash_reference_policy.pytests/test_contextguard_stage2_feasibility.pytests/test_phase_evaluation.py
🚧 Files skipped from review as they are similar to previous changes (6)
- context-guard-kit/bash_reference_policy.py
- plugins/context-guard/bin/bash_reference_policy.py
- packages/context-guard-receipt/bin/launcher.cjs
- tests/test_contextguard_stage2_feasibility.py
- packages/context-guard-receipt/package-files.json
- packages/context-guard-receipt/python/context_guard_receipt/phase_evaluation.py
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
`@packages/context-guard-receipt/tests/contract/test_g015_phase_evaluation_cli.py`:
- Around line 184-188: The CLI contract test should validate stratum identities,
not just the count. In the assertions after parsing completed.stdout in
test_g015_phase_evaluation_cli, compare the returned strata’s stratum values
against the complete expected set s0 through s799, while preserving the existing
count and output-size checks.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 643b05fe-d6ff-40c4-9115-e44d582aa27b
📒 Files selected for processing (11)
context-guard-kit/bash_reference_policy.pycontext-guard-kit/phase_evaluation.pypackages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/python/context_guard_receipt/cli.pypackages/context-guard-receipt/python/context_guard_receipt/phase_evaluation.pypackages/context-guard-receipt/schemas/phase-evaluation-p4.schema.jsonpackages/context-guard-receipt/tests/contract/test_g015_phase_evaluation_cli.pyplugins/context-guard/bin/bash_reference_policy.pytests/test_contextguard_stage2_feasibility.pytests/test_phase_evaluation.py
🚧 Files skipped from review as they are similar to previous changes (10)
- context-guard-kit/bash_reference_policy.py
- plugins/context-guard/bin/bash_reference_policy.py
- packages/context-guard-receipt/schemas/phase-evaluation-p4.schema.json
- packages/context-guard-receipt/python/context_guard_receipt/cli.py
- packages/context-guard-receipt/bin/launcher.cjs
- packages/context-guard-receipt/package-files.json
- tests/test_contextguard_stage2_feasibility.py
- tests/test_phase_evaluation.py
- context-guard-kit/phase_evaluation.py
- packages/context-guard-receipt/python/context_guard_receipt/phase_evaluation.py
Comment on lines
+184
to
+188
| completed = self.run_cli(canonical_json(record)) | ||
|
|
||
| self.assertEqual(completed.returncode, 0, completed.stderr) | ||
| self.assertGreater(len(completed.stdout), 64 * 1024) | ||
| self.assertEqual(len(json.loads(completed.stdout)["strata"]), 800) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the returned stratum identities.
Line 188 checks only the number of strata. The evaluator can drop one input stratum and duplicate another, and this test still passes. Compare the returned stratum values with all s0 through s799 values.
Proposed test fix
self.assertEqual(completed.returncode, 0, completed.stderr)
self.assertGreater(len(completed.stdout), 64 * 1024)
- self.assertEqual(len(json.loads(completed.stdout)["strata"]), 800)
+ result = json.loads(completed.stdout)
+ self.assertEqual(len(result["strata"]), 800)
+ self.assertEqual(
+ {entry["stratum"] for entry in result["strata"]},
+ {f"s{index}" for index in range(800)},
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| completed = self.run_cli(canonical_json(record)) | |
| self.assertEqual(completed.returncode, 0, completed.stderr) | |
| self.assertGreater(len(completed.stdout), 64 * 1024) | |
| self.assertEqual(len(json.loads(completed.stdout)["strata"]), 800) | |
| completed = self.run_cli(canonical_json(record)) | |
| self.assertEqual(completed.returncode, 0, completed.stderr) | |
| self.assertGreater(len(completed.stdout), 64 * 1024) | |
| result = json.loads(completed.stdout) | |
| self.assertEqual(len(result["strata"]), 800) | |
| self.assertEqual( | |
| {entry["stratum"] for entry in result["strata"]}, | |
| {f"s{index}" for index in range(800)}, | |
| ) |
🤖 Prompt for 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.
In
`@packages/context-guard-receipt/tests/contract/test_g015_phase_evaluation_cli.py`
around lines 184 - 188, The CLI contract test should validate stratum
identities, not just the count. In the assertions after parsing completed.stdout
in test_g015_phase_evaluation_cli, compare the returned strata’s stratum values
against the complete expected set s0 through s799, while preserving the existing
count and output-size checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
evaluate phaseCLISafety boundary
nextandlatestremain out of scopeVerification
python3 scripts/prepublish_check.py(1,593 tests; 3 hosted-only skips)Summary by CodeRabbit
New Features
context-guard-receipt evaluate phase --input <file|->.Documentation
Bug Fixes