fix: close architecture review guard gaps

[BigSimmo]

Summary

• Preserve case and POSIX backslash semantics when deriving local project identity on case-sensitive hosts.
• Keep Windows project roots case-insensitive and separator-normalized.
• Extend the architecture graph guard to detect static dynamic imports with options/template literals and singleton self-import cycles.
• Add platform-aware local project ID/port regression coverage.

Why

These are the three valid CodeRabbit findings raised on PR #457. That PR was merged before the corrective push completed, so this focused follow-up applies only those fixes to the merged main tree.

Verification

• tests/architecture-boundaries.test.ts — 6 passed
• tests/local-server-utils.test.ts — 2 passed
• Focused Prettier — passed
• git diff --check — passed
• TypeScript passed before the local offline dependency refresh; after a timed-out npm ci --offline, the worktree install became incomplete and a rerun failed on missing next/lucide-react declarations. Hosted CI's clean install is the authoritative typecheck for this head.

Not run

No browser, provider-backed Supabase/OpenAI, deployment, or production checks were run.

Risk

Low and isolated to local project identity generation and test-only architecture graph parsing. Existing one-argument callers retain their behavior on Windows.

[supabase]

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[coderabbitai]

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 599ed845-d9e2-4ab3-9313-1509088c71e1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Platform-aware project identity utilities now distinguish Windows and POSIX path semantics. Architecture regression tests now parse broader dynamic-import forms and detect runtime self-import cycles, with the review ledger recording the follow-up.

Changes

Identity and architecture guard

Layer / File(s)	Summary
Platform-aware project identity src/lib/local-server-utils.mjs, tests/local-server-utils.test.ts	Project root normalization, hashing, local IDs, and stable ports accept an optional platform and are tested against Windows and Linux path semantics.
Dynamic import and self-cycle detection tests/architecture-boundaries.test.ts, docs/branch-review-ledger.md	Module extraction supports source-text parsing and option-bearing dynamic imports; runtime self-imports are classified as cycles, with both cases covered by tests and recorded in the review ledger.

Estimated code review effort: 3 (Moderate) | ~20 minutes

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Verification Claims	❌ Error	PR notes say tests/Prettier/typecheck/CI scans "passed" but often omit the exact command/check; e.g. "tests — 6 passed" and "hosted CI/SAST/Secret Scan passed".	Rewrite verification notes to name each exact command/check and outcome, e.g. Ran npm run verify:cheap: passed, or Not run: reason.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 70.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (9 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately reflects the main fix: closing architecture review guard gaps.
Description check	✅ Passed	The description covers summary, verification, risk, and relevant caveats, with only optional template items omitted.
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.
Generated And Sensitive Files	✅ Passed	Diff only touches docs/tests/code; no real .env, keys, logs, caches, build artifacts, or other sensitive/generated files appear in the PR.
Risky Git Or Deployment Actions	✅ Passed	PASS: The PR only adds safe review-ledger notes and tests; it does not introduce force-push, reset --hard, branch deletion, destructive clean, or deployment advice.
Supabase Project And Schema Safety	✅ Passed	The touched files are local identity tests/docs; no Supabase project refs, schema, migrations, RLS, or policy files were changed.
Runtime And Package Manager Integrity	✅ Passed	Diff only touches docs and a test file; no package-manager, lockfile, engine-strict, or runtime-version changes were made.
Api Route Failure Handling	✅ Passed	PASS: The PR only changes a doc, local-server utilities, and tests; no API routes, server actions, RAG/search/ingestion/provider code was modified.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/architecture-review-followup

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch codex/architecture-review-followup

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@tests/local-server-utils.test.ts`:
- Around line 22-23: Replace the collision-based assertions for localProjectId
and stableProjectPort with checks that upperCaseRoot and lowerCaseRoot normalize
differently, while verifying each function’s deterministic behavior and valid
output range; alternatively, assert pinned expected fixture values without
requiring distinct outputs.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 06b4b2bb-ca47-4ddd-87a3-543c37941acc

📥 Commits

Reviewing files that changed from the base of the PR and between 2b7e538 and f5deaae.

📒 Files selected for processing (4)

• docs/branch-review-ledger.md
• src/lib/local-server-utils.mjs
• tests/architecture-boundaries.test.ts
• tests/local-server-utils.test.ts

[BigSimmo]

@coderabbitai review

The prior finding was addressed in f08b7f055: the test now checks POSIX normalization differences, deterministic project IDs/ports, ID shape, and valid port range without assuming distinct hash or modulo outputs.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
fd -t f "local-server-utils.test.ts" | xargs -I{} sh -c 'echo "== {} =="; cat -n {}'
git log --oneline -5
git log --oneline --all | grep -i f08b7f0 || true

Length of output: 2649

---

@BigSimmo Confirmed — I checked tests/local-server-utils.test.ts at f08b7f0 and the new "preserves POSIX case and backslash semantics" test now:

• Asserts normalizeProjectRoot differs between case variants and between backslash vs. slash forms (deterministic, non-probabilistic checks).
• Verifies determinism by calling localProjectId/stableProjectPort twice per input and comparing.
• Validates the ID shape via ^clinical-kb:[0-9a-f]{12}$ and the port range 3100–4599.

It no longer assumes distinct hash/modulo outputs across all four inputs, which addresses the flakiness concern from the prior review. Kicking off a fresh review of the PR now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Resolved in f08b7f0: replaced probabilistic collision assertions with deterministic normalization, repeatability, ID-shape, and port-range checks. Focused test passes 2/2; hosted format, lint, typecheck, unit coverage, safety, and build checks are green.