docs: make policy gates enforcement-first#180
Conversation
|
Warning Review limit reached
More reviews will be available in 41 minutes and 45 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefactors doc-drift to README-status checks, adds policy-tests and text-encoding gates in scripts/axis.py and CI, extends unit tests for policy gates, updates CI/PR templates and playbooks to the new Ready/Verification/Retrospective checkpoint model, and normalizes BOM/encoding across many files. ChangesPolicy Gates and Doc Drift Checkpoint Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/doc_drift_domains.py (2)
74-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMatch only the exact endpoint file path, not a stem prefix.
Line 76 builds a stem-prefix regex and Line 77 uses
re.search, which can incorrectly trigger API-status checks for unrelated files that share the same prefix insrc/Axis.Api/Endpoints/.Suggested fix
- for endpoint_file in ENDPOINTS_DIR.glob("*Endpoints.cs"): - stem = endpoint_file.stem[: -len("Endpoints")] - pattern = f"src/Axis\\.Api/Endpoints/{re.escape(stem)}" - if not any(path_matches_pattern(p, pattern) for p in changed_paths): + for endpoint_file in ENDPOINTS_DIR.glob("*Endpoints.cs"): + endpoint_rel = f"src/Axis.Api/Endpoints/{endpoint_file.name}" + if endpoint_rel not in changed_paths: continue domains_to_check.add(module_to_domain_slug(primary_application_module(endpoint_file)))🤖 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 `@scripts/doc_drift_domains.py` around lines 74 - 77, The current loop builds a stem-prefix pattern from ENDPOINTS_DIR by using stem and then checks changed_paths with path_matches_pattern, which can match unrelated files sharing that prefix; update the pattern to match the exact endpoint filename (include the "Endpoints.cs" suffix or anchor the regex to the filename) or use the full endpoint_file.name (e.g., re.escape(endpoint_file.name)) so path_matches_pattern only returns true for the exact path in src/Axis.Api/Endpoints/, and preserve use locations: ENDPOINTS_DIR, endpoint_file, stem, pattern, and path_matches_pattern.
79-79:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle mapping errors here to avoid hard-failing doc-drift execution.
Line 79 can raise
ValueError. Inscripts/axis.py(check_doc_drift), discovery errors are collected butcheck_readme_api_status()is still called, so this can crash the command instead of reporting issues cleanly.Suggested fix
- domains_to_check.add(module_to_domain_slug(primary_application_module(endpoint_file))) + try: + domains_to_check.add(module_to_domain_slug(primary_application_module(endpoint_file))) + except ValueError as exc: + errors.append(str(exc))🤖 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 `@scripts/doc_drift_domains.py` at line 79, The line adding domains_to_check can raise ValueError via primary_application_module or module_to_domain_slug; wrap the call to primary_application_module(endpoint_file) and module_to_domain_slug(...) in a try/except ValueError block inside scripts/doc_drift_domains.py, catch the exception, record the error into the existing discovery_errors collection or log it (instead of re-raising), and continue so check_readme_api_status() won't hard-fail; reference the symbols domains_to_check, primary_application_module, module_to_domain_slug, and discovery_errors when making the change.docs/playbooks/repo-layout-discovery.md (1)
69-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winChecklist A/B are out of sync with the pre-push command requirements.
Line 39-44 requires both
python scripts/axis.py check policy-testsandpython scripts/axis.py check doc-drift, but Line 69 and Line 77 only call outcheck doc-drift.Suggested fix
-- [ ] Run `python scripts/axis.py check doc-drift` and full `dotnet test` on `Axis.sln`. +- [ ] Run `python scripts/axis.py check policy-tests`, `python scripts/axis.py check doc-drift`, and full `dotnet test` on `Axis.sln`. -- [ ] `python scripts/axis.py check doc-drift`. +- [ ] `python scripts/axis.py check policy-tests` and `python scripts/axis.py check doc-drift`.As per coding guidelines: “Keep a single owner per fact—edit one canonical location and link from other docs instead of duplicating commands/steps/versions/paths.”
🤖 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 `@docs/playbooks/repo-layout-discovery.md` around lines 69 - 77, Checklist items for "A" and "B — New use case" are inconsistent: the A checklist requires both "python scripts/axis.py check policy-tests" and "python scripts/axis.py check doc-drift" while the B checklist only lists "python scripts/axis.py check doc-drift"; update the B checklist entries under "B — New use case" to include the missing "python scripts/axis.py check policy-tests" command (and any other pre-push checks present in the A block) so both checklists call the same pre-push checks, and if this command list is duplicated elsewhere, consolidate to a single canonical instruction location and link to it instead of duplicating the commands.
🧹 Nitpick comments (1)
scripts/tests/test_policy_gates.py (1)
163-168: ⚡ Quick winAdd one positive test for endpoint-change enforcement.
This class only verifies the non-trigger path. Please add a case where a changed
*Endpoints.cspath maps to a domain whose README still has| API | ⏳, and assert thatcheck_readme_api_status()returns the expected error.🤖 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 `@scripts/tests/test_policy_gates.py` around lines 163 - 168, Add a positive test method inside TestDocDomainDiscovery that exercises endpoint-change enforcement: create a test (e.g., test_endpoint_change_enforces_doc_activity) that calls doc_drift_domains.check_readme_api_status with a list containing a changed endpoint path matching "*Endpoints.cs" (for example "src/Modules/Identity/Axis.Identity.Application/UsersEndpoints.cs") and set up the README state so it contains the table entry "| API | ⏳" for the mapped domain; assert that check_readme_api_status returns the expected non-empty error list (the specific error string/tuple your code produces) indicating the README API status must be updated. Reference the existing TestDocDomainDiscovery class and the function check_readme_api_status when adding the test. Ensure the test mirrors the style of the existing negative case (using self.assertEqual/assertTrue) and uses the same input/output shape as other tests in the file.
🤖 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 `@CONTRIBUTING.md`:
- Line 22: Update the pointer in the CONTRIBUTING.md sentence that currently
references "repo-layout-discovery.md section D" to point to the repository proto
guidance section or table in docs/playbooks/repo-layout-discovery.md (the
section covering .proto/buf usage and proto checklist) so contributors are
routed correctly; edit the phrase after "see" to reference the correct heading
name or anchor (e.g., "proto guidance" or the exact section title) and ensure
the surrounding text mentioning "python scripts/axis.py generate buf-yaml",
"buf.yaml", and "buf lint" remains unchanged.
---
Outside diff comments:
In `@docs/playbooks/repo-layout-discovery.md`:
- Around line 69-77: Checklist items for "A" and "B — New use case" are
inconsistent: the A checklist requires both "python scripts/axis.py check
policy-tests" and "python scripts/axis.py check doc-drift" while the B checklist
only lists "python scripts/axis.py check doc-drift"; update the B checklist
entries under "B — New use case" to include the missing "python scripts/axis.py
check policy-tests" command (and any other pre-push checks present in the A
block) so both checklists call the same pre-push checks, and if this command
list is duplicated elsewhere, consolidate to a single canonical instruction
location and link to it instead of duplicating the commands.
In `@scripts/doc_drift_domains.py`:
- Around line 74-77: The current loop builds a stem-prefix pattern from
ENDPOINTS_DIR by using stem and then checks changed_paths with
path_matches_pattern, which can match unrelated files sharing that prefix;
update the pattern to match the exact endpoint filename (include the
"Endpoints.cs" suffix or anchor the regex to the filename) or use the full
endpoint_file.name (e.g., re.escape(endpoint_file.name)) so path_matches_pattern
only returns true for the exact path in src/Axis.Api/Endpoints/, and preserve
use locations: ENDPOINTS_DIR, endpoint_file, stem, pattern, and
path_matches_pattern.
- Line 79: The line adding domains_to_check can raise ValueError via
primary_application_module or module_to_domain_slug; wrap the call to
primary_application_module(endpoint_file) and module_to_domain_slug(...) in a
try/except ValueError block inside scripts/doc_drift_domains.py, catch the
exception, record the error into the existing discovery_errors collection or log
it (instead of re-raising), and continue so check_readme_api_status() won't
hard-fail; reference the symbols domains_to_check, primary_application_module,
module_to_domain_slug, and discovery_errors when making the change.
---
Nitpick comments:
In `@scripts/tests/test_policy_gates.py`:
- Around line 163-168: Add a positive test method inside TestDocDomainDiscovery
that exercises endpoint-change enforcement: create a test (e.g.,
test_endpoint_change_enforces_doc_activity) that calls
doc_drift_domains.check_readme_api_status with a list containing a changed
endpoint path matching "*Endpoints.cs" (for example
"src/Modules/Identity/Axis.Identity.Application/UsersEndpoints.cs") and set up
the README state so it contains the table entry "| API | ⏳" for the mapped
domain; assert that check_readme_api_status returns the expected non-empty error
list (the specific error string/tuple your code produces) indicating the README
API status must be updated. Reference the existing TestDocDomainDiscovery class
and the function check_readme_api_status when adding the test. Ensure the test
mirrors the style of the existing negative case (using
self.assertEqual/assertTrue) and uses the same input/output shape as other tests
in the file.
🪄 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: CHILL
Plan: Pro
Run ID: 062bfe33-7aba-4fe3-8488-1e8c93ff2a92
📒 Files selected for processing (16)
.coderabbit.yaml.editorconfig.github/PULL_REQUEST_TEMPLATE.md.github/workflows/build-and-test.ymlCLAUDE.mdCONTRIBUTING.mddocs/ARCHITECTURE.mddocs/README.mddocs/REVIEW_FINDINGS.mddocs/playbooks/agent-checklist.mddocs/playbooks/docs-style.mddocs/playbooks/repo-layout-discovery.mddocs/playbooks/scripts.mdscripts/axis.pyscripts/doc_drift_domains.pyscripts/tests/test_policy_gates.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/playbooks/agent-checklist.md (1)
149-165:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the docs-only example; it contradicts the trigger matrix.
Line 149 requires
policy-tests+doc-driftfor docs/scripts/layout/policy changes, but Line 165 says docs-only means every verification line is “not triggered.” This will cause incorrect checklist claims.Suggested edit
-Example (docs-only): every line `not triggered — no src/, tests/, or frontend/ changes`. +Example (docs-only): policy gate tests + doc drift ran; .NET/frontend lines `not triggered` when no src/tests/frontend code changed.🤖 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 `@docs/playbooks/agent-checklist.md` around lines 149 - 165, The docs-only example is inconsistent with the trigger matrix: update the "Example (docs-only)" section so that when the change touches docs/scripts/layout/policy it does not mark every verification as "not triggered" — specifically set the "policy gate tests" (policy-tests) and "python scripts/axis.py check doc-drift" entries under the "Verification gate self-check" block to show they ran (e.g., "ran") with appropriate reasons, while leaving unrelated checks as "not triggered"; locate the "Verification gate self-check" block and the "Example (docs-only)" paragraph and modify only the policy-tests and doc-drift lines accordingly.
🤖 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 `@scripts/axis.py`:
- Around line 231-245: The MOJIBAKE_MARKERS tuple currently contains a raw
"\u00c2" entry which causes valid "Â" characters to be treated as mojibake;
remove that raw "\u00c2" element from MOJIBAKE_MARKERS (leave the other
mojibake_marker(...) entries intact) so legitimate UTF-8 "Â" is not flagged by
check text-encoding, and run the encoding checks to confirm no false positives.
---
Outside diff comments:
In `@docs/playbooks/agent-checklist.md`:
- Around line 149-165: The docs-only example is inconsistent with the trigger
matrix: update the "Example (docs-only)" section so that when the change touches
docs/scripts/layout/policy it does not mark every verification as "not
triggered" — specifically set the "policy gate tests" (policy-tests) and "python
scripts/axis.py check doc-drift" entries under the "Verification gate
self-check" block to show they ran (e.g., "ran") with appropriate reasons, while
leaving unrelated checks as "not triggered"; locate the "Verification gate
self-check" block and the "Example (docs-only)" paragraph and modify only the
policy-tests and doc-drift lines accordingly.
🪄 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: CHILL
Plan: Pro
Run ID: 94490fb5-a9e0-4efe-ac21-e26fa822dafb
📒 Files selected for processing (46)
.editorconfig.github/PULL_REQUEST_TEMPLATE.md.github/workflows/build-and-test.ymlAxis.slnCLAUDE.mdCONTRIBUTING.mddocs/README.mddocs/REVIEW_FINDINGS.mddocs/playbooks/agent-checklist.mddocs/playbooks/design-gate.mddocs/playbooks/docs-style.mddocs/playbooks/pr-slicing.mddocs/playbooks/repo-layout-discovery.mddocs/playbooks/scripts.mddocs/playbooks/testing.mddocs/wireframes/generate-screens.mjsscripts/axis.pyscripts/tests/test_policy_gates.pysrc/Axis.Api/Properties/launchSettings.jsonsrc/Modules/DataModeling/Axis.DataModeling.Application/Axis.DataModeling.Application.csprojsrc/Modules/DataModeling/Axis.DataModeling.Domain/Axis.DataModeling.Domain.csprojsrc/Modules/DataModeling/Axis.DataModeling.Infrastructure/Axis.DataModeling.Infrastructure.csprojsrc/Modules/DataModeling/Axis.DataModeling.Infrastructure/Migrations/20260526031047_InitialCreate.Designer.cssrc/Modules/DataModeling/Axis.DataModeling.Infrastructure/Migrations/DataModelingDbContextModelSnapshot.cssrc/Modules/FormBuilder/Axis.FormBuilder.Application/Axis.FormBuilder.Application.csprojsrc/Modules/FormBuilder/Axis.FormBuilder.Domain/Axis.FormBuilder.Domain.csprojsrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Axis.FormBuilder.Infrastructure.csprojsrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Migrations/20260526031056_InitialCreate.Designer.cssrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Migrations/FormBuilderDbContextModelSnapshot.cssrc/Modules/Identity/Axis.Identity.Application/Axis.Identity.Application.csprojsrc/Modules/Identity/Axis.Identity.Domain/Axis.Identity.Domain.csprojsrc/Modules/Identity/Axis.Identity.Infrastructure/Migrations/20260603100509_InitialIdentitySchema.Designer.cssrc/Modules/Identity/Axis.Identity.Infrastructure/Migrations/IdentityDbContextModelSnapshot.cssrc/Modules/PageBuilder/Axis.PageBuilder.Application/Axis.PageBuilder.Application.csprojsrc/Modules/PageBuilder/Axis.PageBuilder.Domain/Axis.PageBuilder.Domain.csprojsrc/Modules/PageBuilder/Axis.PageBuilder.Infrastructure/Axis.PageBuilder.Infrastructure.csprojsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Axis.WorkflowBuilder.Application.csprojsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/Axis.WorkflowBuilder.Domain.csprojsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Axis.WorkflowBuilder.Infrastructure.csprojsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/20260526031052_InitialCreate.Designer.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/WorkflowBuilderDbContextModelSnapshot.cssrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Domain/Axis.WorkflowEngine.Domain.csprojsrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Migrations/20260526031100_InitialCreate.Designer.cssrc/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Migrations/WorkflowEngineDbContextModelSnapshot.cssrc/Shared/Axis.Shared.Application/Axis.Shared.Application.csprojsrc/Shared/Axis.Shared.Domain/Axis.Shared.Domain.csproj
✅ Files skipped from review due to trivial changes (38)
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/Axis.WorkflowBuilder.Domain.csproj
- src/Modules/Identity/Axis.Identity.Domain/Axis.Identity.Domain.csproj
- src/Modules/DataModeling/Axis.DataModeling.Application/Axis.DataModeling.Application.csproj
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Axis.WorkflowBuilder.Application.csproj
- src/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Migrations/20260526031100_InitialCreate.Designer.cs
- src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Axis.FormBuilder.Infrastructure.csproj
- src/Modules/Identity/Axis.Identity.Infrastructure/Migrations/20260603100509_InitialIdentitySchema.Designer.cs
- src/Shared/Axis.Shared.Domain/Axis.Shared.Domain.csproj
- src/Modules/DataModeling/Axis.DataModeling.Domain/Axis.DataModeling.Domain.csproj
- src/Modules/PageBuilder/Axis.PageBuilder.Application/Axis.PageBuilder.Application.csproj
- src/Modules/PageBuilder/Axis.PageBuilder.Infrastructure/Axis.PageBuilder.Infrastructure.csproj
- src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Migrations/DataModelingDbContextModelSnapshot.cs
- src/Modules/FormBuilder/Axis.FormBuilder.Domain/Axis.FormBuilder.Domain.csproj
- src/Axis.Api/Properties/launchSettings.json
- src/Modules/Identity/Axis.Identity.Application/Axis.Identity.Application.csproj
- src/Modules/FormBuilder/Axis.FormBuilder.Application/Axis.FormBuilder.Application.csproj
- src/Modules/PageBuilder/Axis.PageBuilder.Domain/Axis.PageBuilder.Domain.csproj
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/20260526031052_InitialCreate.Designer.cs
- src/Modules/WorkflowEngine/Axis.WorkflowEngine.Domain/Axis.WorkflowEngine.Domain.csproj
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Axis.WorkflowBuilder.Infrastructure.csproj
- docs/wireframes/generate-screens.mjs
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/WorkflowBuilderDbContextModelSnapshot.cs
- src/Modules/Identity/Axis.Identity.Infrastructure/Migrations/IdentityDbContextModelSnapshot.cs
- src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Axis.DataModeling.Infrastructure.csproj
- src/Shared/Axis.Shared.Application/Axis.Shared.Application.csproj
- src/Modules/WorkflowEngine/Axis.WorkflowEngine.Infrastructure/Migrations/WorkflowEngineDbContextModelSnapshot.cs
- src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Migrations/20260526031047_InitialCreate.Designer.cs
- Axis.sln
- src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Migrations/20260526031056_InitialCreate.Designer.cs
- docs/playbooks/testing.md
- docs/playbooks/scripts.md
- src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Migrations/FormBuilderDbContextModelSnapshot.cs
- .github/PULL_REQUEST_TEMPLATE.md
- docs/playbooks/design-gate.md
- .editorconfig
- CONTRIBUTING.md
- docs/playbooks/docs-style.md
- docs/playbooks/repo-layout-discovery.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/build-and-test.yml
- scripts/tests/test_policy_gates.py
Summary
Make policy docs enforcement-first: every recurring rule now has an explicit Enforced / Partial / Review-only / Guidance / Not a rule status instead of being described as a blanket gate. Add counterexample tests for custom Python policy gates and run them from both
verifyand the CI Doc drift job. Narrow doc drift to deterministic checks so docs are updated for behavior/spec/status changes, not as token churn for every code-path diff. Add an enforced tracked-text encoding gate (UTF-8without BOM + LF) and normalize existing BOM-bearing files.Linked spec
N/A: governance, documentation, and policy-tooling cleanup; no product use-case AC changed.
Requirements & rules followed
REVIEW_FINDINGS.md, agent checklist, repo-layout/script docs, PR template, CI).scripts/tests/test_policy_gates.py, including encoding failures for BOM/CRLF/invalid UTF-8/mojibake.python scripts/axis.py verifyran green outside sandbox on the latest commit; fulldotnet test Axis.slnremains CI/branch-protection owned.REVIEW_FINDINGS.md; custom gate proof is nowscripts/tests/test_policy_gates.py.TODO/FIXME/NotImplementedException/ placeholder / stub undersrc/,tests/,frontend/src/- confirmed by doc drift.