WS-POL-003-01: add unified guide compilation contracts - #299
Merged
Conversation
📝 WalkthroughWalkthroughThe PR defines bounded project-guide compilation models, immutable ART evidence handling, pre- and post-submission capability projections, fail-closed validation, contract tests, and test-lane integration. ChangesProject-guide compilation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant PreSubmissionCheckerCatalogue
participant PostSubmitPolicy
participant ProjectGuideCompilationContext
participant CompilationValidator
PreSubmissionCheckerCatalogue->>ProjectGuideCompilationContext: provide pre-submission capability projection
PostSubmitPolicy->>ProjectGuideCompilationContext: provide post-submission capability projection
ProjectGuideCompilationContext->>CompilationValidator: validate compilation result
CompilationValidator-->>ProjectGuideCompilationContext: accept or reject result
🚥 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 |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 (3)
backend/tests/test_project_guide_compilation_contracts.py (2)
99-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the hardcoded catalogue size with a source-derived length.
Line 99 pins 26 definitions. Lines 100-102 and the
strict=Truezip on line 103 already prove one projected definition per catalogue entry. Any valid catalogue addition then fails line 99 for no contract reason.♻️ Proposed change
- assert len(projection.definitions) == 26 + assert len(projection.definitions) == len(catalogue.entries)🤖 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 `@backend/tests/test_project_guide_compilation_contracts.py` around lines 99 - 103, Replace the hardcoded 26 in the definition-count assertion with the length derived from catalogue.entries, keeping the existing stable-ID comparison and strict zip validation unchanged.
290-291: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRejection tests do not pin the rule that must reject each case. Both parametrizations assert only that some
ValueErroris raised. A case can then fail through an unrelated validation rule and still pass, which weakens the fail-closed evidence this PR claims.
backend/tests/test_project_guide_compilation_contracts.py#L290-L291: replacematch="capability binding|version"with a per-case expected message fragment passed throughpytest.mark.parametrize.backend/tests/test_project_guide_compilation_contracts.py#L487-L488: add the same per-case expected message fragment topytest.raises(ValueError)for each status, severity, and disposition combination.🤖 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 `@backend/tests/test_project_guide_compilation_contracts.py` around lines 290 - 291, Update backend/tests/test_project_guide_compilation_contracts.py lines 290-291 and 487-488: pass a per-case expected message fragment through each pytest.mark.parametrize entry, and use that fragment in pytest.raises(ValueError, match=...) so every capability/version case and every status, severity, and disposition combination asserts the specific validation rule that rejects it.backend/app/modules/projects/post_submit_policy.py (1)
88-96: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
selectableopens by default for every registered checker.
selectable=name not in default_setgrants model-facing selectability to any name present indefault_checker_registry(). A new registration, including an internal or experimental checker, becomes selectable with no explicit allow decision. The contract elsewhere in this PR is fail-closed, so this default is inconsistent.Consider an explicit frozen selectable snapshot keyed by compiler version, in the same shape as
POST_SUBMIT_DEFAULT_CHECKERS_BY_COMPILER_VERSION. Thenselectablebecomes membership in that snapshot, and an unlisted registration is neither default nor selectable.♻️ Sketch of an explicit selectable snapshot
+POST_SUBMIT_V01_SELECTABLE_CHECKERS = ("check_acceptance_criteria_present",) +POST_SUBMIT_SELECTABLE_CHECKERS_BY_COMPILER_VERSION = MappingProxyType( + {POST_SUBMIT_COMPILER_VERSION: POST_SUBMIT_V01_SELECTABLE_CHECKERS} +) @@ definitions = tuple( PostSubmissionCapabilityDefinition( capability_id=name, capability_version=compiler_version, platform_default=name in default_set, - selectable=name not in default_set, + selectable=name in selectable_set, ) for name in registered )🤖 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 `@backend/app/modules/projects/post_submit_policy.py` around lines 88 - 96, Update the capability-definition construction around PostSubmissionCapabilityDefinition to use an explicit frozen selectable snapshot keyed by compiler version, alongside POST_SUBMIT_DEFAULT_CHECKERS_BY_COMPILER_VERSION. Set selectable only when the checker name is present in the snapshot for compiler_version; keep default membership based on default_set so unlisted registrations are neither default nor selectable.
🤖 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 `@backend/app/interfaces/project_agents.py`:
- Around line 246-264: Update from_material so every material.source_items entry
must have non-empty source_item_id, extraction_usage_id, and
canonical_output_sha256; reject invalid entries instead of filtering them out.
Also reject material when source_items produces no lineage references, and
preserve constructing GuideSourceLineageRef only from complete valid entries.
- Around line 482-509: The validate_project_guide_compilation_result function
must reject results when context.pre_submission_capabilities.available is false.
Add this guard before validating requirements or platform coverage, raising the
established validation error for unavailable mandatory pre-submit coverage;
preserve the existing validation flow when the projection is available.
- Around line 34-40: Update _UNSAFE_MODEL_TEXT to detect credential keywords
followed by whitespace-separated values, not only colon or equals delimiters;
cover Bearer tokens and password/secret/credential/api-key/token forms while
preserving existing matches. Add regression cases for these bare credential
formats in the relevant tests.
---
Nitpick comments:
In `@backend/app/modules/projects/post_submit_policy.py`:
- Around line 88-96: Update the capability-definition construction around
PostSubmissionCapabilityDefinition to use an explicit frozen selectable snapshot
keyed by compiler version, alongside
POST_SUBMIT_DEFAULT_CHECKERS_BY_COMPILER_VERSION. Set selectable only when the
checker name is present in the snapshot for compiler_version; keep default
membership based on default_set so unlisted registrations are neither default
nor selectable.
In `@backend/tests/test_project_guide_compilation_contracts.py`:
- Around line 99-103: Replace the hardcoded 26 in the definition-count assertion
with the length derived from catalogue.entries, keeping the existing stable-ID
comparison and strict zip validation unchanged.
- Around line 290-291: Update
backend/tests/test_project_guide_compilation_contracts.py lines 290-291 and
487-488: pass a per-case expected message fragment through each
pytest.mark.parametrize entry, and use that fragment in
pytest.raises(ValueError, match=...) so every capability/version case and every
status, severity, and disposition combination asserts the specific validation
rule that rejects it.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0674cc40-fb2c-46fc-be0b-9d536a093ffe
📒 Files selected for processing (11)
.agent-loop/initiatives/WS-POL-003-unified-project-guide-compilation/DISCOVERY.md.agent-loop/initiatives/WS-POL-003-unified-project-guide-compilation/PLAN.md.agent-loop/initiatives/WS-POL-003-unified-project-guide-compilation/chunks/WS-POL-003-01-contract-catalogue-projection.md.agent-loop/initiatives/WS-POL-003-unified-project-guide-compilation/reviews/WS-POL-003-01-internal-review-evidence.md.agent-loop/initiatives/WS-POL-003-unified-project-guide-compilation/reviews/WS-POL-003-01-pr-trust-bundle.mdbackend/app/interfaces/project_agents.pybackend/app/modules/checkers/catalogue.pybackend/app/modules/projects/post_submit_policy.pybackend/scripts/run_test_lanes.pybackend/tests/test_ci_test_lanes.pybackend/tests/test_project_guide_compilation_contracts.py
abiorh-claw
self-requested a review
August 8, 2026 15:57
abiorh-claw
approved these changes
Aug 8, 2026
15 tasks
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
2 participants
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.
Chunk
WS-POL-003-01— Unified project-guide compilation contracts and canonical catalogue projections.Goal
Define one strict, bounded contract for compiling immutable verified project-guide material into guide sufficiency, pre-submission policy, and post-submission policy outputs without activating runtime orchestration.
Intent And Planning Context
.agent-loop/initiatives/WS-POL-003-unified-project-guide-compilation/chunks/WS-POL-003-01-contract-and-catalogue.mdWhat Changed
task_lifecycleCI lane.Why It Changed
The guide-sufficiency result and both checker-policy drafts must be derived together from one verified guide snapshot. Separate or mutable inputs could allow policy drift, stale lineage, duplicated catalogues, or unsupported model-authored capability definitions.
Design Chosen
The contract snapshots ART material into deeply immutable canonical values, projects existing phase-owned catalogues without duplicating them, and validates the complete result before any later persistence or activation chunk can consume it.
Alternatives Rejected
Scope Control
Allowed Files Changed
Files Outside Stated Scope
Product Behavior
It does not add a model call, persistence, migration, route, Celery task, authorization action, checker execution, registry mutation, or lifecycle activation.
Evidence
Commands Run
Result Summary
Acceptance Criteria Proof
Test Delta
Tests Added
backend/tests/test_project_guide_compilation_contracts.pycovers immutable snapshots, projections, evidence lineage, bindings, safe/bounded fields, phase ownership, platform coverage, and fail-closed status rules.Tests Modified
backend/tests/test_ci_test_lanes.pyproves the new test module is assigned exactly once.Tests Removed Or Skipped
Internal Reviewer Results
Reviewed code SHA:
e1a7f41e3d138b5954d4fd55f60110506bda0e9fDetailed evidence is recorded under
.agent-loop/initiatives/WS-POL-003-unified-project-guide-compilation/reviews/.External Review
CI And Gate Integrity
Remaining Risks
Follow-Up Work
Human Review Focus
Please inspect deep snapshot immutability, exact phase-owner catalogue reuse, fail-closed platform/default coverage, and the absence of runtime activation.
Human Merge Ownership