Add post-submit checker compiler contract#87
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR makes post-submit checker policies compiler-owned and versioned, routes project service creation/activation through that compiler, updates test fixtures for critical-plus-high blocking severities, and revises documentation and agent-loop tracking to match the new active chunk and policy boundary. ChangesPost-Submit Checker Compiler Implementation
Critical-Severity Blocking Documentation Update
Agent-Loop Workflow Update
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ProjectService
participant PostSubmitPolicyCompiler
participant CheckerRegistry
Client->>ProjectService: create/activate guide with post_submit_checker_policy
ProjectService->>PostSubmitPolicyCompiler: build_project_post_submit_checker_spec
ProjectService->>PostSubmitPolicyCompiler: compile_project_post_submit_checker_spec
PostSubmitPolicyCompiler->>CheckerRegistry: validate execution checkers registered
CheckerRegistry-->>PostSubmitPolicyCompiler: registration result
alt compilation fails
PostSubmitPolicyCompiler-->>ProjectService: PostSubmitCheckerCompilerError
ProjectService-->>Client: PolicySetupBlocked / 422 compilation failed
else compilation succeeds
PostSubmitPolicyCompiler-->>ProjectService: CompiledPostSubmitCheckerPolicy
ProjectService-->>Client: policy_body, policy_hash
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/modules/projects/post_submit_policy.py (1)
242-296: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftLock post-submit policies to a compiler/versioned contract before changing
DEFAULT_DURABLE_CHECKERS.backend/app/modules/projects/post_submit_policy.pyandbackend/app/modules/projects/service.pycurrently requirebody["default_checkers"] == DEFAULT_DURABLE_CHECKERSduringget_active_guide, andPostSubmitCheckerPolicyhas no version field to distinguish older rows. Any future reorder/rename/add/remove will strand already-active guides unless there’s a backfill/recompile path or version gate.🤖 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 242 - 296, The locked post-submit policy contract is tied directly to DEFAULT_DURABLE_CHECKERS, so changing that list will break existing active guides because PostSubmitCheckerPolicy has no versioned contract. Update parse_locked_post_submit_checker_policy_body and the get_active_guide flow to validate against an explicit policy/compiler version rather than the mutable default list, and add a migration/backfill or recompile path for older stored policies so existing rows remain readable when DEFAULT_DURABLE_CHECKERS changes.
🧹 Nitpick comments (3)
backend/app/modules/projects/schemas.py (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider documenting the
Nonesentinel semantics.
blocking_severitiesis nowNone-able whilerequired_checkers/warning_checkersstay non-optionallist[str]. Omitting or explicitly sendingnullresolves to the platform floor (["critical", "high"]") viabuild_project_post_submit_checker_spec, but an explicit[]is rejected as a downgrade. A short field comment would help future readers avoid assumingNoneand[]` are equivalent.🤖 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/schemas.py` around lines 17 - 19, Add a short field comment on blocking_severities in Project schema to document that None means “use the platform floor” while an explicit empty list is treated differently and rejected as a downgrade. Reference the blocking_severities field and build_project_post_submit_checker_spec behavior so future readers understand that None and [] are not equivalent, while required_checkers and warning_checkers remain standard non-optional lists.backend/app/modules/projects/post_submit_policy.py (2)
83-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
build_project_post_submit_checker_specdoesn't validate classifications itself.
build_project_post_submit_checker_speccanonicalizes checker names but never calls_validate_checker_classifications, so duplicate/conflicting/weakened-default checker classifications are only caught later incompile_project_post_submit_checker_spec. Today the only call site (service.py::_post_submit_checker_policy_model) always chains build→compile, so this is currently safe, but the function's docstring implies it returns a valid "constrained" spec on its own — any future caller (e.g., a preview/dry-run endpoint) that usesbuild_*without immediately compiling would silently accept invalid specs.Consider calling
_validate_checker_classifications(required_checkers, warning_checkers)insidebuild_project_post_submit_checker_spectoo, so the function is self-validating.♻️ Proposed fix
def build_project_post_submit_checker_spec( *, project_id: str, guide_version: str, required_checkers: list[str] | None = None, warning_checkers: list[str] | None = None, blocking_severities: list[str] | None = None, ) -> dict[str, Any]: ... + canonical_required = _canonical_checker_names(required_checkers or []) + canonical_warning = _canonical_checker_names(warning_checkers or []) + _validate_checker_classifications(canonical_required, canonical_warning) return { "schema_version": POST_SUBMIT_CHECKER_POLICY_SPEC_SCHEMA_VERSION, "project_id": project_id, "guide_version": guide_version, - "required_checkers": _canonical_checker_names(required_checkers or []), - "warning_checkers": _canonical_checker_names(warning_checkers or []), + "required_checkers": canonical_required, + "warning_checkers": canonical_warning, "blocking_severities": ( list(PLATFORM_BLOCKING_SEVERITIES) if blocking_severities is None else _canonical_blocking_severities(blocking_severities) ), }🤖 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 83 - 166, `build_project_post_submit_checker_spec` should validate checker classifications itself instead of only canonicalizing them. After computing `required_checkers` and `warning_checkers`, call `_validate_checker_classifications` in `build_project_post_submit_checker_spec` so duplicate/conflicting or weakened-default classifications are rejected at build time, matching the “constrained” spec contract. Keep the same canonicalization flow and align the behavior with `compile_project_post_submit_checker_spec`, which already performs this validation.
211-239: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDelete the unused
post_submit_checker_policy_hashhelper. The compile path now builds the body and hashes it inline, and there are no remaining call sites for this wrapper.🤖 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 211 - 239, Remove the unused post_submit_checker_policy_hash helper from post_submit_policy.py, since the policy hash is now computed inline via post_submit_checker_policy_body and canonical_json_hash. Delete the entire function definition and keep the remaining policy body helpers intact; use the function name post_submit_checker_policy_hash to locate it.
🤖 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.
Outside diff comments:
In `@backend/app/modules/projects/post_submit_policy.py`:
- Around line 242-296: The locked post-submit policy contract is tied directly
to DEFAULT_DURABLE_CHECKERS, so changing that list will break existing active
guides because PostSubmitCheckerPolicy has no versioned contract. Update
parse_locked_post_submit_checker_policy_body and the get_active_guide flow to
validate against an explicit policy/compiler version rather than the mutable
default list, and add a migration/backfill or recompile path for older stored
policies so existing rows remain readable when DEFAULT_DURABLE_CHECKERS changes.
---
Nitpick comments:
In `@backend/app/modules/projects/post_submit_policy.py`:
- Around line 83-166: `build_project_post_submit_checker_spec` should validate
checker classifications itself instead of only canonicalizing them. After
computing `required_checkers` and `warning_checkers`, call
`_validate_checker_classifications` in `build_project_post_submit_checker_spec`
so duplicate/conflicting or weakened-default classifications are rejected at
build time, matching the “constrained” spec contract. Keep the same
canonicalization flow and align the behavior with
`compile_project_post_submit_checker_spec`, which already performs this
validation.
- Around line 211-239: Remove the unused post_submit_checker_policy_hash helper
from post_submit_policy.py, since the policy hash is now computed inline via
post_submit_checker_policy_body and canonical_json_hash. Delete the entire
function definition and keep the remaining policy body helpers intact; use the
function name post_submit_checker_policy_hash to locate it.
In `@backend/app/modules/projects/schemas.py`:
- Around line 17-19: Add a short field comment on blocking_severities in Project
schema to document that None means “use the platform floor” while an explicit
empty list is treated differently and rejected as a downgrade. Reference the
blocking_severities field and build_project_post_submit_checker_spec behavior so
future readers understand that None and [] are not equivalent, while
required_checkers and warning_checkers remain standard non-optional lists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0670051b-5300-4a0b-ba66-daa1c062053a
📒 Files selected for processing (28)
.agent-loop/LOOP_STATE.md.agent-loop/WORK_QUEUE.md.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/CHUNK_MAP.md.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/STATUS.md.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/chunks/WS-POL-002-01-post-submit-compiler-contract.md.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/reviews/WS-POL-002-01-internal-review-evidence.md.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/reviews/WS-POL-002-01-pr-trust-bundle.mdbackend/app/modules/projects/post_submit_policy.pybackend/app/modules/projects/schemas.pybackend/app/modules/projects/service.pybackend/scripts/api_contract_e2e.pybackend/tests/test_checkers.pybackend/tests/test_projects.pybackend/tests/test_tasks.pydocs/architecture_checker_framework.mddocs/architecture_data_model.mddocs/architecture_lifecycle_state_machine.mddocs/operations_project_operating_manual.mddocs/operations_queue_policy.mddocs/operations_reviewer_workflow.mddocs/principles.mddocs/product_first_user_flows.mddocs/product_principles.mddocs/roadmap_30_day_master_plan.mddocs/roadmap_pilot_plan.mddocs/roles_permissions.mddocs/template_checker_policy.mdexamples/terminal_benchmark/terminal_benchmark_api_e2e.py
abiorh-claw
approved these changes
Jul 9, 2026
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.
PR Trust Bundle: WS-POL-002-01
Chunk
WS-POL-002-01- Post-Submit Compiler ContractGoal
Introduce the trusted compiler contract that turns a constrained
project-scoped post-submit checker spec into the canonical
PostSubmitCheckerPolicybody and hash.Human-Approved Intent
Post-submit checker setup must follow the same discipline as pre-submit:
agent-derived setup output can propose a constrained specification later, but
Workstream's trusted compiler owns the runtime policy. The checker, not an
agent, evaluates finalized submissions.
This chunk is intentionally compiler-only. Derivation agents, setup-run fields,
approval APIs, and runtime routing hardening remain in later WS-POL-002 chunks.
What Changed
build_project_post_submit_checker_spec.compile_project_post_submit_checker_spec.PostSubmitCheckerCompilerErrorand compiled policy output type.project-specific
required_checkersandwarning_checkers.default_checkersandexecution_checkers.["critical", "high"]as the platform blocking severity floor.duplicates, conflicting required/warning classifications, warning-only
default checker overrides, default-list drift, and severity downgrades.
Design Chosen
The compiler is owned by
backend/app/modules/projects/post_submit_policy.py.It validates against the registered deterministic checker catalog but does not
move logic into
backend/app/modules/checkers/compiler.py, which remains thepre-submit compiler module.
Policy identity remains:
The canonical body contains:
schema_versionproject_idguide_versiondefault_checkersrequired_checkerswarning_checkersexecution_checkersblocking_severitiesAlternatives Rejected
preserve subsystem separation.
required_checkers:rejected because default-only projects should be clear and project-specific
additions should stay separate from platform defaults.
high: rejected because the runner treatsboth
criticalandhighas blocking, and project policy cannot weaken theplatform floor.
Scope Control
No Alembic migrations, ORM models, repositories, task modules, frontend,
payment/reputation, blockchain, or agent runtime code changed.
The only script/example changes align existing live-drill bootstrap
blocking_severitiespayloads with the compiler floor.Product Behavior
Project setup can now create a post-submit checker policy through the trusted
compiler path. Default-only projects remain valid, but finalized submissions
will still execute all platform durable post-submit checkers once tasks lock
that project policy.
Public setup errors stay generic. Raw checker/spec details are not returned to
workers or project setup callers.
Acceptance Criteria Proof
execution_checkers.policy_body.default_checkersmust exactly equalDEFAULT_DURABLE_CHECKERS.blocking_severities: [],["critical"], and["high"]failclosed.
sha256(canonical_json(policy_body)).downgrades, conflicting classifications, and hash mismatches.
backend/app/modules/checkers/compiler.py.Tests/Checks Run
Result summary:
Reviewer Results
Internal review evidence:
.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/reviews/WS-POL-002-01-internal-review-evidence.mdReviewed code SHA:
4c3dd191de7917d3959d5e01287977d318c0e43bReviewed at:
2026-07-09T16:02:30ZExternal Review
External review is pending. CodeRabbit, GitHub Actions, and any human review
comments must be recorded separately in:
.agent-loop/initiatives/WS-POL-002-post-submit-checker-foundation/reviews/WS-POL-002-01-external-review-response.mdRemaining Risks
exist in the checker runner. Future severity model changes must be versioned
or consolidated deliberately.
Human Review Focus
post-submit policy.
run.
Human Merge Ownership
Only the user can approve and merge this PR. Codex must not merge it without
explicit user approval for that specific PR.
Summary by CodeRabbit