fix(drift): resolve cited paths in every frame they are legitimately written in (0.1.0-alpha.14) - #44
Conversation
…written in Drift resolved every reference from the repo root plus two fixed ancestors, and measurement put the false-positive floor near 60%. Instruction files cite paths in frames that resolution never tried. scripts/lib/path-resolution.ts collects those frames — workspace package roots read from package.json `workspaces` and pnpm-workspace.yaml, ancestor roots, declared package names, and tsconfig `paths` aliases — and a reference is only reported when it resolves NOWHERE. Measured on the eight fixture repos, path issues 87 -> 30: vision 6 -> 1 watchtower 7 -> 0 currychat 1 -> 0 lc-classic-starter 3 -> 0 360training 43 -> 19 atlas 15 -> 9 recall 11 -> 1 forge 1 -> 1 All four must-stay-quiet repos are quiet. `~` is matched against tsconfig aliases BEFORE $HOME, because both meanings are live: currychat's is an alias, lc-classic-starter's is a home, and expanding unconditionally only trades one false-positive class for another. Two corrections to the brief's diagnosis, both found by measuring: The `mdc:` fix is not about the scheme. PATH_PATTERN is `\b`-anchored and there is no word boundary between a delimiter and a leading dot, so the dot is lost whether or not `mdc:` is stripped — my first attempt removed the scheme and changed nothing. Restoring a dropped leading dot at resolution time is what actually fixes it, and it covers every dot-prefixed path, not just Cursor's. `@360training/ui` is not a declared package name and so is not a false positive. That workspace publishes `@t360/*`; the only `@360training/*` package is cloudflare-proxy. It is a stale specifier and stays flagged, correctly. Not taken: making pattern-doc references non-issues. A golden test already encodes the opposite contract — pattern docs skip cross-project examples but KEEP missing local doc refs — and one forge example is not enough to overturn a tested decision. forge's `data/action-log.md` therefore stays flagged. pathIssues is still NOT wired into the verdict. 28 of the remaining 30 sit in two repos and belong to classes nobody has sorted yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe package version changes to ChangesAlpha.14 release and path drift resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/lib/path-resolution.ts`:
- Around line 129-139: Update ancestorRoots to retain only ancestor directories
containing a repository or workspace marker, such as package.json or .git,
instead of adding every parent. Preserve the existing depth and
filesystem-boundary limits, and add a fixture covering an intermediate non-root
parent with a matching path so it remains reported as drift.
- Around line 154-183: Update collectAliases to process every declared
compilerOptions.paths alias, not only tilde-prefixed entries, and resolve each
target relative to compilerOptions.baseUrl when set or the containing tsconfig
directory otherwise. Preserve alias prefixes and add fixtures covering both an
explicit baseUrl and the unset-baseUrl fallback.
🪄 Autofix
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
Run ID: cce5bbb0-c7c9-4ec5-8790-fec40a4070a0
📒 Files selected for processing (12)
README.mddocs-site/public/llms-full.txtdocs-site/src/content/docs/reference/cli.mddocs/byok-trust-model.mddocs/first-user-proof-packet.mddocs/first-user-proof.mddocs/getting-started.mddocs/proofs/current-outside-tester-send-packet.mdpackage.jsonscripts/drift-detect.tsscripts/lib/path-resolution.test.tsscripts/lib/path-resolution.ts
Two CodeRabbit findings on #44, both valid. ancestorRoots accepted every parent for four levels while its own comment claimed a package.json/.git test. On these hosts ~/projects holds many repos, so a genuinely missing path could resolve against an unrelated sibling and vanish from the report — trading a false positive for a false negative, which is the worse direction. The documented test is now the implemented one. resolvesSomewhere applied only tilde-prefixed aliases, so `@/file.ts` and `#/lib/y` were still reported as drift, and collectAliases ignored baseUrl. Every declared alias now applies, resolved against baseUrl when set. `~` is still tried as a home directory, but only after aliases. Neither cost coverage: fixture totals 30 -> 29, with 360training 19 -> 17 from the alias generalization and no repo regressing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Takes asks 1–5.
pathIssuesis still NOT wired into the verdict — reasoning below.Drift resolved every reference from the repo root plus two fixed ancestors; your measurement put the FP floor near 60%.
scripts/lib/path-resolution.tsnow collects the frames a reference may legitimately resolve in — workspace package roots (fromworkspaces/pnpm-workspace.yaml), ancestor roots, declared package names, tsconfigpathsaliases — and reports only references that resolve nowhere.Path issues on the fixture repos, 87 → 30
~is matched against tsconfig aliases before$HOME, since both meanings are live in the fleet.Two corrections to the diagnosis
Ask 4 is not about the scheme.
PATH_PATTERNis\b-anchored, and there is no word boundary between a delimiter and a leading dot — so the dot is lost whether or notmdc:is stripped. My first attempt removed the scheme and changed nothing. Restoring a dropped leading dot at resolution time is what actually fixes it, and it covers every dot-prefixed path, not just Cursor's.@360training/uiis not a false positive. That workspace publishes@t360/*; the only@360training/*package iscloudflare-proxy. It's a stale specifier and stays flagged, correctly. Ask 2 clears the genuinely-declared names (@wt/uietc.) and leaves this one.§3 calls
data/action-log.mdstays flagged.Why not wire scoring
28 of the 30 remaining sit in two repos, in exactly the classes §4 says are unsorted. Wiring now would import that.
Verify
lint · format:check · release:pin --check · typecheck · 513 pass / 0 fail · verify:self-audit-proof.
🤖 Generated with Claude Code