chore: prevent doc + diagram drift (file-move + doc-and-diagram + lean-CLAUDE.md)#114
Merged
Merged
Conversation
Three more layers on top of cf27628 (parts 2-4 of the file-move prevention loop). Same compounding-loop principle, broader coverage: (a) Broken-COPY audit — Dockerfile* COPY/ADD source paths that don't exist in the build context. Catches the kind of drift that left Dockerfile.catalog referencing the pre-VSA-collapse 4-project layout for months after PR #31. Skips `--from=<stage>` cross-stage copies and wildcards (resolved at build time, not against the repo tree). (b) Diagram-pair audit — every docs/*.excalidraw must have its sibling docs/*.svg and vice versa. Reviewers look at the .svg on github.com to understand the system; .excalidraw is the editable source. If one exists without the other, the diagram review surface is broken. Does NOT verify the .svg matches what would be regenerated from the .excalidraw source (that needs Playwright in CI — separate, heavier gate). The pair-existence check is the cheap mechanical floor. (c) CLAUDE.md "Doc-and-diagram discipline" rule. Sibling to the file-move rule. Encodes that docs and diagrams are the REVIEW SURFACE, not byproducts — when reviewers look at the system, they read docs/architecture.md and look at docs/nextaurora-architecture.svg. If those are stale, every review reasons against a fiction. Names concrete pairings: AppHost.cs ↔ architecture.md/svg, Extensions.cs middleware order ↔ service-request-flow.svg, EF/cache/outbox changes ↔ perf-doc + their sibling diagrams. (d) CodeRabbit path_instructions for NextAurora.AppHost/AppHost.cs and NextAurora.ServiceDefaults/Extensions.cs — when topology or middleware-order code changes, flag missing paired doc/diagram updates at review time. PR-description waivers acceptable when the deferred update is named in a tracking issue. All three new mechanical guards (broken-link from #112, broken-COPY, diagram-pair) smoke-tested locally on the current tree → exit 0. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ity) Third commit on this branch — the lean-CLAUDE.md pass on top of the file-move (cf27628) + doc-and-diagram (4e1ba94) prevention layers. CLAUDE.md drops from 358 → 301 lines (~16% smaller) by moving deep-dive content out and leaving headlines + links. Every rule preserved; nothing the AI needs to know got dropped. Continuous Rule Encoding — 6 surfaces → 5: - GitHub Issues moved out of the encoding loop (it's a deferral mechanism, not encoding — closed issues aren't re-read in future sessions) - Mirror update in docs/dev-loop.md (table + prose) - Diagram redesigned with 5 surfaces in a clean row, simpler tier boxes (no crammed tool lists), CLAUDE.md annotated "(kept lean)" to reinforce the discipline visually New "one-paragraph max per rule" discipline on CLAUDE.md surface 1: - "If a rule needs more than ~6 lines, the rule stays as a bolded headline + one-paragraph summary in CLAUDE.md; detail moves to docs/ or skills/" - Test: "could this rule + its rationale fit on one screen?" CI size guard (build job): - CLAUDE.md soft-warning at 400 lines, hard-fail at 500 - Mechanical floor on bloat regression Performance Rules trim (~30 dense bullets → 22 headlines + links): - Rules themselves unchanged (project-not-map, Task.WhenAll, outbox-atomic, Guid v7, AsSpan, Dapper escape hatch, etc.) - Deep-dive paragraphs → docs/performance-and-data-correctness.md + dotnet-performance skill (both already exist) Observability section trim (~78 lines → 21): - Kept 3 always-on traps: HTTP middleware order, Wolverine middleware instance-methods, outbox-outside-handler atomicity - Moved "how it works" detail → docs/architecture.md "Cross-Cutting Concerns" and "Event-Driven Architecture" (both already cover this) Verification: - wc -l CLAUDE.md: 301 (well under the 400/500 size budget) - broken-link audit on CLAUDE.md: exit 0 - diagram-pair audit: exit 0 - broken-COPY audit: exit 0 - All rule headlines preserved; rules with paraphrases in code comments ("See CLAUDE.md") still align with canonical wording Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
WalkthroughAdds a five-surface continuous rule encoding system: a PostToolUse file-move detection hook, CodeRabbit path-pairing rules, CI audits (CLAUDE size budget and Dockerfile COPY checks), and consolidated documentation updates and diagrams. ChangesContinuous Rule Encoding System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 @.claude/scripts/check-file-moves.sh:
- Around line 67-73: The grep that builds the matches variable is currently
scanning all markdown files and will pick up the allowlisted
`.claude/audits/INDEX.md`; update the matches assignment in check-file-moves.sh
to exclude that specific file when searching for "$old_path" under "$REPO_ROOT"
(e.g., add an --exclude or filter out `.claude/audits/INDEX.md` from the grep
results) so links from INDEX.md are not flagged as drift.
In @.claude/settings.json:
- Around line 65-71: The hook configuration adds a Bash hook script (matcher
"Bash" with command
"/Users/joshuadell/NovaCraft/.claude/scripts/check-file-moves.sh") that handles
both git mv and git rm, but the permissions list only allows Bash(git rm *);
update the permissions to also include Bash(git mv *) so tool-driven sessions
can perform renames—locate the permissions array in the same
.claude/settings.json where Bash(git rm *) is declared and add an entry for
Bash(git mv *).
In @.github/workflows/ci.yml:
- Around line 119-129: Add strict shell mode to the Bash run blocks that perform
the CLAUDE.md line-count audits by inserting set -euo pipefail at the very top
of each run: | block (specifically the block that computes lines=$(wc -l <
CLAUDE.md) and the other audit blocks referenced around 178-206 and 222-239);
this ensures the line-count checks and early exits fail loudly and the workflow
job fails on errors, undefined variables, or pipeline errors.
In `@CLAUDE.md`:
- Around line 287-294: The fenced code block containing the middleware order
(the lines with app.UseExceptionHandler(); app.UseAuthentication();
app.UseMiddleware<CorrelationIdMiddleware>(); app.UseAuthorization();) violates
MD031 because there is no blank line before the opening ```csharp and after the
closing ```; fix by inserting a single blank line immediately before the opening
fence and a single blank line immediately after the closing fence so the code
block is separated from surrounding text and markdownlint MD031 is satisfied.
- Around line 191-199: CLAUDE.md has exceeded the 500-line hard limit and is
blocking CI; trim CLAUDE.md down to only headline rules with one-paragraph
summaries and add "See docs/..." pointers for any rule that needs more detail,
moving full explanations into docs/ (or into .claude/skills/ or a paired
.claude/agents/architecture-reviewer.md entry as appropriate); update
.coderabbit.yaml path_instructions if you need file-scoped guidance so reviews
catch the rule, and verify CLAUDE.md falls under the 500-line budget so the
build will pass.
- Line 250: The in-page anchor `[Observability → Transactional
Outbox](`#transactional-outbox-wolverine`)` is broken; either update the link
target to the actual heading id used elsewhere in CLAUDE.md (search for the
"Observability" or "Transactional Outbox" heading and use its exact fragment) or
add a matching heading with id/slug `transactional-outbox-wolverine`; edit the
line containing that link so the fragment matches the document’s heading slugs
(keep the link text "Observability → Transactional Outbox" intact).
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: eb2b957f-8696-4064-943c-c926a43b5e26
⛔ Files ignored due to path filters (1)
docs/dev-loop-scaffolding.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
.claude/scripts/check-file-moves.sh.claude/settings.json.coderabbit.yaml.github/workflows/ci.ymlCLAUDE.mddocs/dev-loop-scaffolding.excalidrawdocs/dev-loop.md
Comment on lines
+67
to
+73
| matches=$(grep -rln --fixed-strings "$old_path" \ | ||
| --include='*.md' --include='*.cs' --include='*.props' --include='*.csproj' \ | ||
| --include='*.yml' --include='*.yaml' --include='*.sh' \ | ||
| --include='Dockerfile*' \ | ||
| "$REPO_ROOT" 2>/dev/null \ | ||
| | head -30 \ | ||
| || true) |
There was a problem hiding this comment.
Exclude the documented allowlist file from drift matches.
Line 67 currently scans all markdown files, which will also flag .claude/audits/INDEX.md; that file is explicitly allowlisted and should be skipped to avoid false positives.
Suggested patch
- matches=$(grep -rln --fixed-strings "$old_path" \
+ matches=$(grep -rln --fixed-strings "$old_path" \
--include='*.md' --include='*.cs' --include='*.props' --include='*.csproj' \
--include='*.yml' --include='*.yaml' --include='*.sh' \
--include='Dockerfile*' \
"$REPO_ROOT" 2>/dev/null \
+ | grep -v "^${REPO_ROOT}/.claude/audits/INDEX.md$" \
| head -30 \
|| true)As per coding guidelines, “Allowlist: .claude/audits/INDEX.md … do NOT flag links from INDEX.md as drift.”
🤖 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 @.claude/scripts/check-file-moves.sh around lines 67 - 73, The grep that
builds the matches variable is currently scanning all markdown files and will
pick up the allowlisted `.claude/audits/INDEX.md`; update the matches assignment
in check-file-moves.sh to exclude that specific file when searching for
"$old_path" under "$REPO_ROOT" (e.g., add an --exclude or filter out
`.claude/audits/INDEX.md` from the grep results) so links from INDEX.md are not
flagged as drift.
Comment on lines
+65
to
+71
| { | ||
| "matcher": "Bash", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "/Users/joshuadell/NovaCraft/.claude/scripts/check-file-moves.sh" | ||
| } |
There was a problem hiding this comment.
git mv path is configured in hook logic but not allowed in permissions.
The new hook handles both git mv and git rm, but only Bash(git rm *) is permitted. Add Bash(git mv *) so the rename branch can actually execute in tool-driven sessions.
Suggested patch
"Bash(git checkout *)",
"Bash(git pull *)",
+ "Bash(git mv *)",
"Bash(git rm *)",🤖 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 @.claude/settings.json around lines 65 - 71, The hook configuration adds a
Bash hook script (matcher "Bash" with command
"/Users/joshuadell/NovaCraft/.claude/scripts/check-file-moves.sh") that handles
both git mv and git rm, but the permissions list only allows Bash(git rm *);
update the permissions to also include Bash(git mv *) so tool-driven sessions
can perform renames—locate the permissions array in the same
.claude/settings.json where Bash(git rm *) is declared and add an entry for
Bash(git mv *).
Comment on lines
+119
to
+129
| run: | | ||
| lines=$(wc -l < CLAUDE.md) | ||
| echo "CLAUDE.md is $lines lines." | ||
| if [ "$lines" -gt 500 ]; then | ||
| echo "::error file=CLAUDE.md::CLAUDE.md exceeds 500-line hard limit (current: $lines). Move detail to docs/ or .claude/skills/ and leave a headline + link. See CLAUDE.md 'Continuous Rule Encoding' surface 1." | ||
| exit 1 | ||
| fi | ||
| if [ "$lines" -gt 400 ]; then | ||
| echo "::warning file=CLAUDE.md::CLAUDE.md is approaching the size budget ($lines lines; soft limit 400, hard limit 500). Consider moving detail out." | ||
| fi | ||
|
|
There was a problem hiding this comment.
Add strict shell mode to the new CI audit run blocks.
The newly added multiline bash steps do not set set -euo pipefail. Please add it at the top of each run: | block so audit failures are surfaced reliably.
Suggested patch pattern
- name: CLAUDE.md size budget
run: |
+ set -euo pipefail
lines=$(wc -l < CLAUDE.md)
echo "CLAUDE.md is $lines lines."
...
- name: Broken-COPY audit — Dockerfile source paths
run: |
+ set -euo pipefail
fail=0
while IFS= read -r dockerfile; do
...
- name: Diagram-pair audit — every .excalidraw needs a sibling .svg
run: |
+ set -euo pipefail
fail=0
while IFS= read -r excalidraw; do
...As per coding guidelines for .github/workflows/*.yml, “DO flag … missing set -euo pipefail in bash run blocks.”
Also applies to: 178-206, 222-239
🤖 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 @.github/workflows/ci.yml around lines 119 - 129, Add strict shell mode to
the Bash run blocks that perform the CLAUDE.md line-count audits by inserting
set -euo pipefail at the very top of each run: | block (specifically the block
that computes lines=$(wc -l < CLAUDE.md) and the other audit blocks referenced
around 178-206 and 222-239); this ensures the line-count checks and early exits
fail loudly and the workflow job fails on errors, undefined variables, or
pipeline errors.
Comment on lines
+287
to
294
| - **HTTP middleware order — strict.** `CorrelationIdMiddleware` runs AFTER `UseAuthentication` (it reads `context.User` to populate `UserId`) and BEFORE `UseAuthorization` (so 401/403 denials log with the user). Canonical order in `MapDefaultEndpoints` (see [Extensions.cs](NextAurora.ServiceDefaults/Extensions.cs)): | ||
| ```csharp | ||
| app.UseExceptionHandler(); | ||
| app.UseAuthentication(); | ||
| app.UseMiddleware<CorrelationIdMiddleware>(); | ||
| app.UseAuthorization(); | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Add blank lines around the fenced code block at Line 288.
This trips markdownlint (MD031) and is a quick cleanup.
Suggested fix
- **HTTP middleware order — strict.** `CorrelationIdMiddleware` runs AFTER `UseAuthentication` (it reads `context.User` to populate `UserId`) and BEFORE `UseAuthorization` (so 401/403 denials log with the user). Canonical order in `MapDefaultEndpoints` (see [Extensions.cs](NextAurora.ServiceDefaults/Extensions.cs)):
+
```csharp
app.UseExceptionHandler();
app.UseAuthentication();
app.UseMiddleware<CorrelationIdMiddleware>();
app.UseAuthorization();</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
- **HTTP middleware order — strict.** `CorrelationIdMiddleware` runs AFTER `UseAuthentication` (it reads `context.User` to populate `UserId`) and BEFORE `UseAuthorization` (so 401/403 denials log with the user). Canonical order in `MapDefaultEndpoints` (see [Extensions.cs](NextAurora.ServiceDefaults/Extensions.cs)):
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 288-288: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 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 `@CLAUDE.md` around lines 287 - 294, The fenced code block containing the
middleware order (the lines with app.UseExceptionHandler();
app.UseAuthentication(); app.UseMiddleware<CorrelationIdMiddleware>();
app.UseAuthorization();) violates MD031 because there is no blank line before
the opening ```csharp and after the closing ```; fix by inserting a single blank
line immediately before the opening fence and a single blank line immediately
after the closing fence so the code block is separated from surrounding text and
markdownlint MD031 is satisfied.
The prior commit (5c938b2) trimmed CLAUDE.md from 358→301 lines and added "See docs/..." links. An audit found the content I trimmed wasn't actually in those destination docs — I'd deleted it, not moved it. This commit does the real moves. New file: docs/observability-and-context-propagation.md - Section-aligned with CLAUDE.md "Observability & Context Propagation" - Full mechanism + headers/baggage mapping table + sources - HTTP middleware order with the canonical 4-line code example - Wolverine pipeline scope detail (pipeline order: validation → context → handler → AutoApplyTransactions) - Wolverine envelope context extraction mechanism - Transactional Outbox config + outbox-outside-handler atomicity trap with the canonical safe wrapper code block - Structured logging scope hygiene - Event Replay note Additions to docs/performance-and-data-correctness.md: - New "## Additional always-on patterns" section after "The 14 always-on rules" — patterns that don't fit a single-rule shape - Non-sargable predicates + EmailNormalized normalize-at-write-time pattern - Task.WhenAll parallel awaits with three caveats (dependent ops, shared DbContext, multi-failure observability) - Long-running work / 202 Accepted pattern with two atomicity paths and cloud-managed alternatives (Durable Functions, Step Functions, Temporal) - Fan-out on message bus + MaxDegreeOfParallelism throttle - Guid.CreateVersion7 with the time-decodable trade-off - AsSpan ref-struct + async-boundary constraint CLAUDE.md link redirects (no rule text changes): - "Long-running work" → docs/performance-and-data-correctness.md "Long-running work belongs on the message bus" (new section) - "Observability & Context Propagation" mechanism → docs/observability-and-context-propagation.md (new file) - "Outbox outside a Wolverine handler" wrapper → docs/observability-and-context-propagation.md anchor (new file) Naming convention: section-aligned filenames where new docs are created (observability-and-context-propagation.md mirrors the CLAUDE.md heading). Existing docs kept where they already align by topic (performance-and-data-correctness.md). Verification: - wc -l CLAUDE.md: 301 (unchanged; under 400 soft / 500 hard budget) - broken-link audit: exit 0 (full repo) - All trimmed content now lives in either the new doc or the existing perf doc; nothing the AI needs to know got dropped Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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 (2)
CLAUDE.md (2)
287-294:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd blank lines around the fenced code block.
Markdownlint MD031 requires blank lines before and after fenced code blocks to visually separate them from surrounding text.
📝 Proposed fix
- **HTTP middleware order — strict.** `CorrelationIdMiddleware` runs AFTER `UseAuthentication` (it reads `context.User` to populate `UserId`) and BEFORE `UseAuthorization` (so 401/403 denials log with the user). Canonical order in `MapDefaultEndpoints` (see [Extensions.cs](NextAurora.ServiceDefaults/Extensions.cs)): + ```csharp app.UseExceptionHandler(); app.UseAuthentication(); app.UseMiddleware<CorrelationIdMiddleware>(); app.UseAuthorization();
- Wolverine middleware classes must use instance methods.
opts.Policies.AddMiddleware<T>()only discoversBefore/After/Finally(andAsyncvariants) as instance methods on a public class with a public constructor. Static methods aren't discovered →InvalidWolverineMiddlewareExceptionat host startup. Suppress S2325 ("should be static") with aJustificationreferencing this rule.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@CLAUDE.mdaround lines 287 - 294, The fenced C# code block showing the
middleware order needs blank lines before and after the triple-backtick fence to
satisfy MD031; edit the documentation around the snippet that mentions
CorrelationIdMiddleware / MapDefaultEndpoints / Extensions.cs so there is an
empty line immediately above the openingcsharp and an empty line immediately below the closingso the code block is visually separated from surrounding
list items and text.</details> --- `250-250`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix broken anchor reference.** The link `[Observability → Transactional Outbox](`#transactional-outbox-wolverine`)` points to a non-existent in-page fragment. The content moved to the new observability doc in this PR. <details> <summary>🔗 Proposed fix</summary> ```diff -- **Outbox atomicity**: entity write + outbox-row write commit in the same transaction. Prefer one `SaveChanges` call; otherwise `BeginTransactionAsync` explicitly. See [Observability → Transactional Outbox](`#transactional-outbox-wolverine`) for the non-handler-code trap. +- **Outbox atomicity**: entity write + outbox-row write commit in the same transaction. Prefer one `SaveChanges` call; otherwise `BeginTransactionAsync` explicitly. See [docs/observability-and-context-propagation.md "Outbox outside a Wolverine handler"](docs/observability-and-context-propagation.md#outbox-outside-a-wolverine-handler--atomicity-trap) for the non-handler-code trap. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` at line 250, The in-page anchor "[Observability → Transactional Outbox](`#transactional-outbox-wolverine`)" is broken because the content was moved; update that Markdown link in CLAUDE.md by replacing the fragment target "`#transactional-outbox-wolverine`" with the correct new reference to the observability doc/fragment introduced in this PR (use the new document filename and its "transactional outbox" anchor or the top-level Observability doc link), ensuring the link points to the moved "Transactional Outbox" section so readers land on the correct content. ``` </details> </blockquote></details> </blockquote></details>🤖 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 `@docs/performance-and-data-correctness.md`: - Around line 170-176: Insert a blank line immediately before the opening fenced code block that contains the C# snippet so the file conforms to Markdownlint MD031; locate the block starting with the fence that contains GetUserAsync, GetOrdersAsync, and GetNotificationsAsync and add one empty line above the ```csharp opening fence. --- Outside diff comments: In `@CLAUDE.md`: - Around line 287-294: The fenced C# code block showing the middleware order needs blank lines before and after the triple-backtick fence to satisfy MD031; edit the documentation around the snippet that mentions CorrelationIdMiddleware / MapDefaultEndpoints / Extensions.cs so there is an empty line immediately above the opening ```csharp and an empty line immediately below the closing ``` so the code block is visually separated from surrounding list items and text. - Line 250: The in-page anchor "[Observability → Transactional Outbox](`#transactional-outbox-wolverine`)" is broken because the content was moved; update that Markdown link in CLAUDE.md by replacing the fragment target "`#transactional-outbox-wolverine`" with the correct new reference to the observability doc/fragment introduced in this PR (use the new document filename and its "transactional outbox" anchor or the top-level Observability doc link), ensuring the link points to the moved "Transactional Outbox" section so readers land on the correct content.🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID:
9fc6cc52-45c3-4156-a69b-3493c814349c📒 Files selected for processing (3)
CLAUDE.mddocs/observability-and-context-propagation.mddocs/performance-and-data-correctness.md
Comment on lines
+170
to
+176
| **Anti-shape:** | ||
| ```csharp | ||
| var user = await GetUserAsync(id, ct); | ||
| var orders = await GetOrdersAsync(otherId, ct); // doesn't depend on user | ||
| var notifications = await GetNotificationsAsync(ct); // doesn't depend on either | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Add blank line before fenced code block.
Markdownlint MD031 requires blank lines before and after fenced code blocks. The closing fence has a blank line after it (line 176), but the opening fence at line 171 is missing a blank line before it.
📝 Proposed fix
Sequential `await`s serialize latency for free. Async makes a single wait non-blocking; it does not make a *sequence* of waits cheap. When a handler makes N independent I/O calls — N gRPC requests to different services, N HTTP calls to different external APIs, N queries against *different* DbContexts (one per service) — sequential `await`s pay the sum of all latencies, while `Task.WhenAll` pays the max.
**Anti-shape:**
+
```csharp
var user = await GetUserAsync(id, ct);
var orders = await GetOrdersAsync(otherId, ct); // doesn't depend on user
var notifications = await GetNotificationsAsync(ct); // doesn't depend on either</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 171-171: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
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/performance-and-data-correctness.md around lines 170 - 176, Insert a
blank line immediately before the opening fenced code block that contains the C#
snippet so the file conforms to Markdownlint MD031; locate the block starting
with the fence that contains GetUserAsync, GetOrdersAsync, and
GetNotificationsAsync and add one empty line above the ```csharp opening fence.
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
emeraldleaf
added a commit
that referenced
this pull request
Jun 5, 2026
#116) Three doc sites had drift from the recent encoding-loop work: README.md (line 18 "Two AI reviewers" callout) - Old: lists only .claude/skills/ as what Claude Code reads - New: lists the full .claude/ folder (agents, skills, slash commands, hook scripts, hook + permission wiring) — the meta-layer that gets ported between projects - ADDED a new "Continuous Rule Encoding" callout right after, introducing the loop + 3-tier enforcement spectrum + lean-CLAUDE.md discipline (~300 lines, CI soft cap 400, hard fail 500) + the 5 encoding surfaces + cross-links to /feature-spec, /article-audit, /check-rules, /new-feature-slice, /sync-status. Points at docs/dev-loop.md for full mechanics and docs/dev-loop-scaffolding.svg for the scaffolding diagram. docs/dev-loop.md (Slash commands table) - Was missing /feature-spec (just landed in #115) and /article-audit (landed earlier this week) - Added both with their actual one-line descriptions docs/dev-loop.svg (rendered diagram people see on GitHub) - Stage 1 "PostToolUse" hooks list: added check-file-moves.sh (the hook added in #114 for git-mv/git-rm detection) - Stage 1 "Slash commands" list: added /feature-spec and /article-audit alongside the existing 3; shifted Agents/Skills/Secondary-reviewer positions down by 54px to make room - Encoding loop "Encode in:" footer: was 4 surfaces (CLAUDE.md + .coderabbit.yaml + arch-reviewer Pattern Checklist + skill), now 5 surfaces (added "docs + paired diagrams" as surface 5, shortened labels to fit). Title now reads "Encode in 1+ of 5 surfaces" to clarify the surface count What this is NOT: - Not a full dev-loop.svg regeneration — the SVG also has other minor staleness (CLAUDE.md size annotation "25 KB" is outdated since the trim; integration tests list only shows Catalog + Order while CI now runs 4 slices). Targeted surgery here, not a rewrite. Verification: - broken-link guard: exit 0 on the changed markdown files - All cross-references resolve Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
emeraldleaf
added a commit
that referenced
this pull request
Jun 5, 2026
…n Triangle (#119) Net result of auditing all 5 articles in Kapil Viren Ahuja's IDSD vs SDD series (Activated Thinker, March–May 2026). 10 method additions across 3 surfaces. One originally-proposed change (Connections section) dropped after re-audit — Ahuja himself walked it back in "The Anatomy of Intent." CLAUDE.md "Debugging Discipline" (+ 2 rules, 4 lines net): - Presence in the loop, not approval at the gate — distinguishes pattern-conforming features (gate review fine) from non-pattern-conforming features (stay present during build). Tied to `/feature-spec` Significance Check. - Continue is the verb that gets you in trouble. Build is not. — prototypes need token budget + stop-time, set up front. From The Trap: "Nobody approves that. Nobody ever approves that. It approves itself, one month at a time." .claude/commands/feature-spec.md (6 additions): - Step 1 prepended with Value gate (3 CXO questions): who needs this, would we still build it at engineering-time cost, who owns saying no. From The Trap. - Goal section: two-implementations test — if only one implementation can satisfy this, you wrote a spec disguised as a goal. From IDSD. - Acceptance criteria: constraints-vs-failure-conditions decision rule callout. From IDSD. - Affects: Upstream dependencies (assumptions that could shift) — names the load-bearing context the spec depends on, so the AI flags during build if any of it moves. From "SDD Will Collapse." - New Non-functional constraints section (optional, 5-7 lines max, business language, not implementation patterns). From IDSD's anatomy of Intent. - Step 5 added: Hole-test — final completeness check before shipping spec. From IDSD. - Existing Closing the loop renumbered to step 6. docs/dev-loop.md (2 additions): - "This is a method, not a harness" section after At a glance — encoding loop is the method; Claude Code, CodeRabbit, GitHub Actions, Spec Kit, BMAD, Kiro, Garura are harnesses. Adopting a harness without a method is the default failure mode today. - "Lineage and grounding" section before Source links — Larman/Basili 2003, Ostroff/Makalsky/Paige 2004 XP, METR 2025, Anthropic Sept 2025 postmortem, OpenAI Symphony April 2026, Uber 2026 budget burn, Ahuja's IDSD series. Includes the decay-warning rule and the encoding loop's contribution to the lineage (cross-feature compounding). Also corrected stale "six destinations" in At a glance → "five surfaces" to match the 5-surface encoding model we shipped in #114. Verification: - wc -l CLAUDE.md: 305 (was 301; soft cap 400, hard 500 — comfortably under) - broken-link audit on full repo: exit 0 - All rule headlines linkable via /check-rules Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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
Three-commit branch encoding the prevention loop for the drift class surfaced by PR #112 (CatalogService Clean → VSA collapse fallout). Plus a discipline pass to keep CLAUDE.md from silently bloating going forward.
The full prevention loop, layered
What changed in CLAUDE.md (third commit)
Diagram
`docs/dev-loop-scaffolding.{excalidraw,svg}` — redesigned for readability:
Verification
What this is NOT
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores