Skip to content

chore: docs/CI governance — fix drift, trim duplication, add guardrails#51

Merged
phuongnse merged 5 commits into
mainfrom
chore/docs-ci-governance-fixes
May 22, 2026
Merged

chore: docs/CI governance — fix drift, trim duplication, add guardrails#51
phuongnse merged 5 commits into
mainfrom
chore/docs-ci-governance-fixes

Conversation

@phuongnse

@phuongnse phuongnse commented May 22, 2026

Copy link
Copy Markdown
Owner

Summary

Three-commit cleanup of the docs and CI governance layer. Commit 1 fixes concrete drift (removed unit-tests.slnf still referenced 6×, conflicting Gate-paste guidance across PR template / CONTRIBUTING / agent-checklist, missing nCrunchTemp ignore, no concurrency on CI, no cross-module SQL gate). Commit 2 removes duplication and one speculative section (ARCHITECTURE.md "Real-Time Updates / SignalR" admitted as not implemented; README "Roadmap" dup of PRODUCT_VISION; CONTRIBUTING ~50% dup of CLAUDE.md). Commit 3 locks in guardrails so this class of rot can't return without CI catching it (Lychee link check, Dependabot for action pinning, speculation guard in drift script, docs-style.md anti-pattern playbook, removed catalog tables from CLAUDE.md).

Net: +287 / −227 lines. The doctrine surface area shrank; the automation surface area grew. Every new rule in docs-style.md is paired with a CI/script check — no "process about process".

Linked spec

Docs-governance change. Touches: CLAUDE.md, CONTRIBUTING.md, docs/ARCHITECTURE.md, docs/README.md, docs/PRODUCT_VISION.md, docs/TECH_STACK.md, docs/playbooks/agent-checklist.md, docs/playbooks/process.md, new docs/playbooks/docs-style.md, .github/PULL_REQUEST_TEMPLATE.md, .github/workflows/build-and-test.yml, new .github/dependabot.yml, new lychee.toml, scripts/check-doc-drift.sh, .gitignore.

Requirements & rules followed

  • Spec → code — N/A, docs/CI-only PR (no src/ changes, no AC to map).
  • Gate 0 — N/A (no code shipped).
  • Gate 1dotnet * and npm * not triggered (no src/, tests/, or frontend/ changes). dotnet format --verify-no-changes not triggered. ./scripts/check-doc-drift.sh ran green locally on the branch.
  • Gate 2 — docs same-PR: CLAUDE.md docs index, docs/README.md playbooks table, and agent-checklist all updated to reference the new docs-style.md. No epic feature touched (no module code changed).
  • Gate 3 — retrospective ran; durable rules captured in docs/playbooks/docs-style.md and in the new check-doc-drift.sh guards (speculation + new-raw-SQL review + no-new-TODO + renamed-handler tests). One direction change mid-task — initial cross-module SQL guard scanned the whole tree (too noisy); narrowed to diff-added lines only, documented in commit message.
  • No new TODO / FIXME / NotImplementedException / placeholder / stub under src/, tests/, frontend/src/ — verified by the new drift-script guard (which scans added lines in the diff).

New guardrails introduced

  • CI job Markdown link check (lychee, offline mode) — internal links and #anchors must resolve.
  • CI: Dependabot for github-actions — auto-pins @v2@<sha> # v2.4.1 on first run, weekly bumps thereafter.
  • CI: concurrency group on CI workflow — cancels superseded PR runs.
  • CI: docker info pre-check on .NET job — Testcontainers failures surface early.
  • Drift script: speculation guard (ARCHITECTURE.md must not contain "Not yet implemented" / "planned design" / "Will be wired" / "To be implemented" / "Coming soon"). Smoke-tested with a fixture.
  • Drift script: new-raw-SQL review (cross-module P0 enforcement, diff-added lines only).
  • Drift script: no-new-TODO/FIXME/stub (diff-added lines only, all changed src/, tests/, frontend/src/).
  • Drift script: handles renamed handler files (R git status), not just added.

Summary by CodeRabbit

  • Documentation

    • Added a docs style guide and expanded playbooks with clearer authoring, ownership, and doc-drift workflows
    • Updated architecture, tech-stack, product-vision, contributing guidance, and tightened PR template checklist (including stricter gate instructions and no new TODO/FIXME/stubs)
  • Chores

    • Enabled automated dependency updates for CI actions and added CI link/anchor validation
    • Hardened CI concurrency/permissions and improved doc-drift tooling to enforce hygiene, test-presence, and ignore additional temp files

Review Change Stack

phuongnse and others added 3 commits May 22, 2026 13:48
- process.md: drop 6 references to removed `unit-tests.slnf`; use full
  `dotnet test` on `Axis.sln` consistent with CLAUDE.md Gate 1.
- PR template: rebuild around Summary + Linked spec + Requirements; ticks
  now mirror Gates 0-3 ordering with explicit no-new-TODO checkbox.
- agent-checklist: dedupe PR-body guidance; clarify that paste blocks are
  for agent walk-through, not the PR description; drop manual TODO grep
  from Gate 1 (now enforced by drift script).
- CONTRIBUTING: align gate language with template ("walk locally, tick in
  PR"); note bash requirement for drift script on Windows.
- CLAUDE.md: same bash note; remove duplicate TODO grep now that the
  drift script enforces it.
- check-doc-drift.sh: scan *added* diff lines for new raw-SQL calls in
  module code (cross-module P0 guard) and for new TODO/FIXME/stub
  markers; handle renamed handlers, not just added.
- build-and-test.yml: add `concurrency` group to cancel superseded PR
  runs; tighten `permissions: contents: read`; add `docker info`
  pre-check on the .NET job so Testcontainers failures surface early.
- .gitignore: ignore nCrunch temp project files.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- ARCHITECTURE.md: rewrite as a thin architectural overview that points
  at owners instead of duplicating them. Drop the SignalR "Real-Time
  Updates" section (admitted as not implemented; speculation pretending
  to be reference), the source-tree dup (now owned by CLAUDE.md), and
  the version column on the Containers table (now owned by
  TECH_STACK.md). Net: 150 → 88 lines.
- docs/README.md: drop the Roadmap section (Phase 1/2/3 dup of
  PRODUCT_VISION § MVP Scope). Replace with a "single source of truth
  per topic" ownership table so every contributor can answer "which doc
  do I edit?" in one glance.
- CONTRIBUTING.md: rebuild as a short first-contributor pointer
  (branch/commit conventions + Gates link). Removes ~50 lines of
  duplication with CLAUDE.md and agent-checklist.md.
- PRODUCT_VISION.md: drop "Success Metrics" — aspirational numbers
  nobody is measuring or testing against.
- TECH_STACK.md: mark SignalR row as ⏳ not yet wired so the table no
  longer implies the hub is live.
- agent-checklist.md: update Gate 2 "Project structure" pointer from
  ARCHITECTURE.md to CLAUDE.md § Solution tree (the new owner).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Doctrine without automation rots — every rule here is paired with a CI
or script check.

- CI: add Markdown link check job (lychee, offline mode via lychee.toml).
  Catches broken internal links and #anchor drift on every PR. External
  http(s) intentionally not blocking — third-party 503s don't belong in
  PR CI.
- CI: add Dependabot config for github-actions. Auto-pins `@v2` to SHA
  on first run and keeps actions fresh weekly. Canonical alternative to
  hand-rolled SHA hunting.
- drift script: speculation guard. Fails if docs/ARCHITECTURE.md
  contains phrases that indicate forward-looking content ("Not yet
  implemented", "planned design", "Will be wired"). Reference docs
  describe what EXISTS; forward-looking belongs in PROGRESS.md or epic
  feature files. Smoke-tested with a fixture.
- New playbook: docs/playbooks/docs-style.md. Short anti-pattern
  checklist + size budgets + when-to-create-vs-absorb. The doctrine
  exists so the CI rules feel justified, not arbitrary.
- CLAUDE.md: remove the mini Tech stack and Modules tables — both were
  duplicates of TECH_STACK.md / epics README and a known drift source.
  CLAUDE.md now owns *rules*, not catalogs. Add docs-style.md to the
  Docs index.
- agent-checklist: note the two CI-only gates (Doc drift, Link check)
  so agents don't try to run them locally.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Repository governance updates: CI concurrency and Markdown link checking (lychee), Dependabot for GitHub Actions, enhanced scripts/check-doc-drift.sh guards (raw-SQL, handler-test, TODO/FIXME/speculation), updated contributor docs and PR template, a docs playbook, architecture clarifications, and small config fixes.

Changes

Documentation Governance and CI Standards

Layer / File(s) Summary
CI infrastructure and link-checking foundation
.github/workflows/build-and-test.yml, .github/dependabot.yml, lychee.toml
Workflow concurrency/permissions, docker info check for .NET job, lychee-based Markdown internal link/anchor verification wired into CI, Dependabot config for weekly GitHub Actions updates, and lychee.toml settings.
Doc-drift script enhancements: raw-SQL, handler-test checks, and hygiene guards
scripts/check-doc-drift.sh
P0 raw-SQL guard for src/Modules, include renamed handlers in handler→test enforcement, and scan only added lines for TODO/FIXME/NotImplementedException/placeholders and speculative wording in ARCHITECTURE.md.
Contributor workflow guidelines: templates, CONTRIBUTING, and agent checklist
.github/PULL_REQUEST_TEMPLATE.md, CONTRIBUTING.md, docs/playbooks/agent-checklist.md
PR template tightens Gate 1/2/3 expectations and forbids new TODO/FIXME/stubs under src//tests/. CONTRIBUTING.md refactors to a docs-first condensed workflow. Agent checklist updates doc-drift and CI-only gate behavior.
Documentation standards playbook and docs ownership governance
docs/playbooks/docs-style.md, docs/README.md
New docs-style playbook with authoring conventions, single-owner rules, anti-patterns, edit checklists; docs/README.md adds playbook entry and ownership table.
Architecture documentation refactoring: scope, cross-doc links, and clarity
CLAUDE.md, docs/ARCHITECTURE.md, docs/TECH_STACK.md, docs/PRODUCT_VISION.md
ARCHITECTURE.md clarifies scope, defers tech details to TECH_STACK.md, adds cross-module communication constraints and schema-per-tenant approach, condenses auth/workflow to epic references. CLAUDE.md updated to require ./scripts/check-doc-drift.sh before push. TECH_STACK and PRODUCT_VISION small edits.
Process documentation: standardized full-solution test execution
docs/playbooks/process.md
Standardizes TDD gates to run dotnet test against full Axis.sln with "zero errors, zero warnings"; removes unit-tests.slnf coupling.
Configuration: NCrunch temporary directories
.gitignore
Adds patterns to ignore NCrunch temporary directories and .csproj files.
Epic README and patterns anchor fixes
docs/epics/E07-page-builder/README.md, docs/playbooks/patterns*.md
Marks E07 feature files as pending and corrects in-page anchor targets for Result-vs-exceptions links.

Sequence Diagram

sequenceDiagram
  participant Developer
  participant GitHub as GitHub Actions
  participant Lychee as lycheeverse/lychee-action
  participant Drift as scripts/check-doc-drift.sh
  Developer->>GitHub: open/push PR (triggers workflow)
  GitHub->>Lychee: run Markdown link/anchor checks (uses ./lychee.toml)
  GitHub->>Drift: run doc-drift script
  Lychee-->>GitHub: link/anchor results
  Drift-->>GitHub: drift/hygiene results
  GitHub->>Developer: CI status (pass/fail)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • phuong-labs/axis#42: Related cross-module raw-SQL rules and doc-drift enforcement.
  • phuong-labs/axis#48: Overlapping updates to PR templates, agent checklist, and doc-drift tooling.
  • phuong-labs/axis#49: Part of the same docs-governance change set; similar CLAUDE/playbook/script edits.

Poem

🐰 A rabbit hops through docs so bright,

With gates and guards and standards tight,
Raw SQL checked, no TODOs hide,
The lychee sniffs each broken guide,
Now the repo reads with clearer sight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely summarizes the main changes: documentation and CI governance improvements focused on fixing drift, removing duplication, and adding guardrails.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/docs-ci-governance-fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
docs/TECH_STACK.md (1)

19-19: ⚡ Quick win

Prefer current-state wording over future-state promise in the stack table.

This line mixes intended behavior and current implementation status. Consider phrasing it as present-state only (e.g., capability available, no hub registered yet) and keep roadmap intent in epics/PROGRESS.

🤖 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/TECH_STACK.md` at line 19, Update the SignalR row in the TECH_STACK
table to describe only the current state: keep the capability ("Real-time
updates") and state that no hub is registered yet (e.g., "capability available;
no `*Hub.cs` in `src/`"), removing future-facing promises like "Not yet wired"
or roadmap language so the table reflects present implementation status only.
🤖 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 @.github/workflows/build-and-test.yml:
- Line 93: Replace the mutable tag for the GitHub Action usage (currently
referenced as lycheeverse/lychee-action@v2) with a pinned full commit SHA;
locate the step that uses "lycheeverse/lychee-action@v2" and change the
reference to "lycheeverse/lychee-action@<full-commit-sha>" (use the action
repository’s latest commit SHA for the v2 release) so the workflow is pinned to
an immutable commit.

In `@docs/ARCHITECTURE.md`:
- Line 26: The Architecture doc's API Server row contains speculative language
("SignalR hub planned for execution push updates"); update the "**API Server**"
table cell to reflect only current state by removing "planned" wording—either
delete the SignalR mention or move it to a parenthetical pointing to the
epic/progress doc (e.g., "SignalR hub for execution push updates — see
epic/progress doc") so the table remains strictly current-state; edit the table
row containing "**API Server**" accordingly.
- Around line 58-62: The fenced code block showing the PostgreSQL schema diagram
lacks a language tag, causing markdownlint MD040 failures; edit the block
containing the lines "PostgreSQL", "├── public schema           # organizations,
subscription_plans, users, roles", and "└── tenant_{orgId} schemas  #
per-tenant: models, workflows, executions, …" and add an explicit language tag
(e.g., ```text) after the opening backticks so the block becomes ```text ... ```
to satisfy the linter.

---

Nitpick comments:
In `@docs/TECH_STACK.md`:
- Line 19: Update the SignalR row in the TECH_STACK table to describe only the
current state: keep the capability ("Real-time updates") and state that no hub
is registered yet (e.g., "capability available; no `*Hub.cs` in `src/`"),
removing future-facing promises like "Not yet wired" or roadmap language so the
table reflects present implementation status only.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f30a7318-1e77-4aad-837c-6050990a92a7

📥 Commits

Reviewing files that changed from the base of the PR and between 34bc8d9 and 2e088f5.

📒 Files selected for processing (15)
  • .github/PULL_REQUEST_TEMPLATE.md
  • .github/dependabot.yml
  • .github/workflows/build-and-test.yml
  • .gitignore
  • CLAUDE.md
  • CONTRIBUTING.md
  • docs/ARCHITECTURE.md
  • docs/PRODUCT_VISION.md
  • docs/README.md
  • docs/TECH_STACK.md
  • docs/playbooks/agent-checklist.md
  • docs/playbooks/docs-style.md
  • docs/playbooks/process.md
  • lychee.toml
  • scripts/check-doc-drift.sh
💤 Files with no reviewable changes (1)
  • docs/PRODUCT_VISION.md

Comment thread .github/workflows/build-and-test.yml Outdated
Comment thread docs/ARCHITECTURE.md Outdated
Comment thread docs/ARCHITECTURE.md Outdated
phuongnse and others added 2 commits May 22, 2026 14:38
Two pre-existing bugs that the new Markdown link check job exposed on PR
#51 — fix in the same PR so the gate lands green.

- patterns.md / patterns-index.md: anchor link to
  `#result-pattern-vs-exceptions----when-to-use-what` (4 dashes) was
  wrong. GitHub's slugger collapses " — " (space-emdash-space) to two
  dashes, not four. Correct anchor:
  `#result-pattern-vs-exceptions--when-to-use-what`. Fixed in 4 spots.
- docs/epics/E07-page-builder/README.md: feature rows linked to
  features/F0N-*.md files that don't exist (E07 is Phase 2, not
  started). Per docs-style.md "empty TODO sections" anti-pattern,
  converted to plain text with a callout explaining files will be
  authored when the epic begins.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Four review comments — all valid; fixed plus one durable improvement.

- workflows/build-and-test.yml: pin lycheeverse/lychee-action to full
  SHA (v2.8.0 = 8646ba30...). Dependabot will keep it fresh; pinning
  now closes the gap between merge and first Dependabot run.
- ARCHITECTURE.md API Server row: drop "planned for execution push
  updates" speculation; point to E06 epic where the real spec lives.
  Reference docs describe what exists.
- ARCHITECTURE.md PostgreSQL diagram: add `text` language tag to
  fenced block (MD040).
- TECH_STACK.md SignalR row: rephrase as pure current state
  ("capability available; no `*Hub.cs` currently registered in
  `src/`") instead of "Not yet wired" which implied a forward
  promise. Link to PROGRESS and E06 for status / epic acceptance.
- check-doc-drift.sh speculation guard: broaden pattern to catch the
  class of "planned X" / "Will be X" the bot just exposed — the
  earlier `planned design` literal was too narrow. Smoke-tested with
  a fixture containing "planned for push".

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
scripts/check-doc-drift.sh (1)

150-156: ⚡ Quick win

Make speculation matching case-insensitive to avoid guardrail blind spots.

Line 156 uses case-sensitive grep -nE, so lowercase variants like will be and coming soon are missed by the current pattern. The verification confirms that grep -nE with the current mixed-case pattern fails to match "This will be added later." while grep -nEi catches it.

Suggested patch
-SPEC_PATTERN='Not yet|\bplanned\b|Will be|To be implemented|Coming soon|in the future'
+SPEC_PATTERN='not yet|\bplanned\b|will be|to be implemented|coming soon|in the future'
@@
-  if matches="$(grep -nE "${SPEC_PATTERN}" "${target}" 2>/dev/null)"; then
+  if matches="$(grep -nEi "${SPEC_PATTERN}" "${target}" 2>/dev/null)"; then
🤖 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/check-doc-drift.sh` around lines 150 - 156, The grep call in the loop
that checks SPEC_PATTERN against files is case-sensitive (grep -nE
"${SPEC_PATTERN}" ...) so lowercase variants are missed; update the check to
perform case-insensitive matching by either enabling grep's -i flag (change grep
-nE to grep -nEi) or make the regex itself case-insensitive (prefix pattern with
(?i)); ensure SPEC_PATTERN remains the same and only the grep invocation or
pattern modifier is adjusted so matches like "will be" and "coming soon" are
detected.
🤖 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.

Nitpick comments:
In `@scripts/check-doc-drift.sh`:
- Around line 150-156: The grep call in the loop that checks SPEC_PATTERN
against files is case-sensitive (grep -nE "${SPEC_PATTERN}" ...) so lowercase
variants are missed; update the check to perform case-insensitive matching by
either enabling grep's -i flag (change grep -nE to grep -nEi) or make the regex
itself case-insensitive (prefix pattern with (?i)); ensure SPEC_PATTERN remains
the same and only the grep invocation or pattern modifier is adjusted so matches
like "will be" and "coming soon" are detected.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ccde0c6-c2e5-4580-b975-c7e5ecbc00e2

📥 Commits

Reviewing files that changed from the base of the PR and between 7b558af and 0a259c5.

📒 Files selected for processing (4)
  • .github/workflows/build-and-test.yml
  • docs/ARCHITECTURE.md
  • docs/TECH_STACK.md
  • scripts/check-doc-drift.sh
✅ Files skipped from review due to trivial changes (1)
  • docs/TECH_STACK.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant