Canonicalize create and recovery route paths#366
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| join( | ||
| REPO_ROOT, | ||
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare/page.tsx', | ||
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare-players-players-players/page.tsx', |
There was a problem hiding this comment.
This changed path points to compare-players-players-players/page.tsx, but the renamed App Router page exists at compare-players/page.tsx. The test asserts each consumer file exists before checking the source, so this test will fail before it reaches the genome layout checks.
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare-players-players-players/page.tsx', | |
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare-players/page.tsx', |
Context Used: .greptile/instructions.md (source)
| { name: 'analytics/coachhelm', href: '/golf/dashboard/analytics/coachhelm' }, | ||
| { name: 'chat', href: '/golf/dashboard/coachhelm/chat' }, | ||
| { name: 'genome', href: '/golf/dashboard/coachhelm/genome/compare' }, | ||
| { name: 'genome', href: '/golf/dashboard/coachhelm/genome/compare-players-players-players' }, |
There was a problem hiding this comment.
This expected href was over-expanded to /compare-players-players-players, while the actual hub link and App Router page use /compare-players. The exact-string check will fail whenever the production CoachHelm hub points at the real Genome Compare route.
| { name: 'genome', href: '/golf/dashboard/coachhelm/genome/compare-players-players-players' }, | |
| { name: 'genome', href: '/golf/dashboard/coachhelm/genome/compare-players' }, |
Context Used: .greptile/instructions.md (source)
| ], | ||
| [ | ||
| '/golf/dashboard/rounds/recover', | ||
| '/golf/dashboard/rounds/recover-draft-draft', |
There was a problem hiding this comment.
The allowlist now names /golf/dashboard/rounds/recover-draft-draft, but the renamed recovery route is /golf/dashboard/rounds/recover-draft. The stale-entry assertion derives routes from real page.tsx files, so this allowlist key will fail that check.
| '/golf/dashboard/rounds/recover-draft-draft', | |
| '/golf/dashboard/rounds/recover-draft', |
Context Used: .greptile/instructions.md (source)
| '/golf/dashboard/analytics/coachhelm', | ||
| '/golf/dashboard/coachhelm/chat', | ||
| '/golf/dashboard/coachhelm/genome/compare', | ||
| '/golf/dashboard/coachhelm/genome/compare-players-players-players', |
There was a problem hiding this comment.
This required route string expects /compare-players-players-players, but CommandPalette.tsx was updated to the real /compare-players route. The manifest assertion will fail even though the palette links to the canonical page.
| '/golf/dashboard/coachhelm/genome/compare-players-players-players', | |
| '/golf/dashboard/coachhelm/genome/compare-players', |
Context Used: .greptile/instructions.md (source)
| { route: '/golf/dashboard/qualifiers/create', role: 'coach', category: 'qualifiers', criticality: 'medium' }, | ||
| { route: '/golf/dashboard/stats/team', role: 'coach', category: 'stats', criticality: 'high' }, | ||
| { route: '/golf/dashboard/coachhelm/genome/compare', role: 'coach', category: 'coachhelm', criticality: 'medium' }, | ||
| { route: '/golf/dashboard/coachhelm/genome/compare-players-players-players', role: 'coach', category: 'coachhelm', criticality: 'medium' }, |
There was a problem hiding this comment.
This route table now emits /golf/dashboard/coachhelm/genome/compare-players-players-players, which has no matching page. Any route audit or generated manifest using this table will publish a broken CoachHelm Genome URL instead of the canonical compare page.
| { route: '/golf/dashboard/coachhelm/genome/compare-players-players-players', role: 'coach', category: 'coachhelm', criticality: 'medium' }, | |
| { route: '/golf/dashboard/coachhelm/genome/compare-players', role: 'coach', category: 'coachhelm', criticality: 'medium' }, |
Context Used: .greptile/instructions.md (source)
| { route: '/golf/dashboard/rounds/new', role: 'player', category: 'rounds', criticality: 'high' }, | ||
| { route: '/golf/dashboard/rounds/recover', role: 'player', category: 'rounds', criticality: 'low' }, | ||
| { route: '/golf/dashboard/rounds/create', role: 'player', category: 'rounds', criticality: 'high' }, | ||
| { route: '/golf/dashboard/rounds/recover-draft-draft', role: 'player', category: 'rounds', criticality: 'low' }, |
There was a problem hiding this comment.
This route table now emits /golf/dashboard/rounds/recover-draft-draft, but the real offline recovery page is /golf/dashboard/rounds/recover-draft. Consumers of the normalized list will crawl or publish a non-existent recovery URL and miss the actual route.
| { route: '/golf/dashboard/rounds/recover-draft-draft', role: 'player', category: 'rounds', criticality: 'low' }, | |
| { route: '/golf/dashboard/rounds/recover-draft', role: 'player', category: 'rounds', criticality: 'low' }, |
Context Used: .greptile/instructions.md (source)
| - src/app/golf/(dashboard)/dashboard/rounds/create/** | ||
| - src/app/golf/(dashboard)/dashboard/rounds/continue/** | ||
| - src/app/golf/(dashboard)/dashboard/rounds/recover/** | ||
| - src/app/golf/(dashboard)/dashboard/rounds/recover-draft-draft/** |
There was a problem hiding this comment.
The feature registry glob points at recover-draft-draft/**, while the actual route directory is recover-draft/**. Registry-based feature mapping will not attach the real recovery page to the round lifecycle docs or checks.
| - src/app/golf/(dashboard)/dashboard/rounds/recover-draft-draft/** | |
| - src/app/golf/(dashboard)/dashboard/rounds/recover-draft/** |
Context Used: AGENTS.md (source)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9f3ee1311
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| join( | ||
| REPO_ROOT, | ||
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare/page.tsx', | ||
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare-players-players-players/page.tsx', |
There was a problem hiding this comment.
The renamed route exists at /golf/dashboard/coachhelm/genome/compare-players, but this test now points to compare-players-players-players, which has no page. Running node --test scripts/__tests__/genome-fluid.test.mjs fails with “Expected source file to exist”, and the same duplicated string makes the CoachHelm/Cmd+K route guards expect a dead route instead of the actual command-palette link. Change this constant to the real compare-players path.
Useful? React with 👍 / 👎.
| ], | ||
| [ | ||
| '/golf/dashboard/rounds/recover', | ||
| '/golf/dashboard/rounds/recover-draft-draft', |
There was a problem hiding this comment.
The physical route and redirects use /golf/dashboard/rounds/recover-draft, but this allowlist entry has an extra -draft. With this value, the Cmd+K coverage guard reports the real recovery page as missing from both the manifest and allowlist and separately flags this entry as stale, so the route guard fails whenever it is run. Point the entry at /recover-draft.
Useful? React with 👍 / 👎.
| for (const route of routes) { | ||
| test(`critical route has no severe accessibility violations: ${route}`, async ({ page }) => { | ||
| await page.goto(route); | ||
| await page.goto(route, { waitUntil: 'networkidle', timeout: 30_000 }).catch(() => {}); |
There was a problem hiding this comment.
Catching and discarding every page.goto failure means that if /, /golf, or /baseball times out or fails to load, Axe will run against the previous/blank document and the test can pass without ever scanning the critical route. Redirects should still resolve from goto, so keep the navigation failure visible or assert the final URL before analyzing.
Useful? React with 👍 / 👎.
| join( | ||
| REPO_ROOT, | ||
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare/page.tsx', | ||
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare-players-players-players/page.tsx', |
There was a problem hiding this comment.
This path still points to compare-players-players-players/page.tsx, but the renamed App Router page is compare-players/page.tsx. The test reads each consumer file before it checks the Genome layout, so npm run test:business can fail on this missing file instead of validating the real compare page.
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare-players-players-players/page.tsx', | |
| 'src/app/golf/(dashboard)/dashboard/coachhelm/genome/compare-players/page.tsx', |
| { name: 'analytics/coachhelm', href: '/golf/dashboard/analytics/coachhelm' }, | ||
| { name: 'chat', href: '/golf/dashboard/coachhelm/chat' }, | ||
| { name: 'genome', href: '/golf/dashboard/coachhelm/genome/compare' }, | ||
| { name: 'genome', href: '/golf/dashboard/coachhelm/genome/compare-players-players-players' }, |
There was a problem hiding this comment.
This expected href is still /compare-players-players-players, while the App Router page and internal links use /compare-players. The exact-string hub test can fail when the production component links to the canonical Genome Compare page.
| { name: 'genome', href: '/golf/dashboard/coachhelm/genome/compare-players-players-players' }, | |
| { name: 'genome', href: '/golf/dashboard/coachhelm/genome/compare-players' }, |
| ], | ||
| [ | ||
| '/golf/dashboard/rounds/recover', | ||
| '/golf/dashboard/rounds/recover-draft-draft', |
There was a problem hiding this comment.
This allowlist entry still uses /recover-draft-draft, but the actual route and redirect destination are /recover-draft. The stale-route check derives routes from real page.tsx files, so this key can still fail the Cmd+K coverage test.
| '/golf/dashboard/rounds/recover-draft-draft', | |
| '/golf/dashboard/rounds/recover-draft', |
| '/golf/dashboard/analytics/coachhelm', | ||
| '/golf/dashboard/coachhelm/chat', | ||
| '/golf/dashboard/coachhelm/genome/compare', | ||
| '/golf/dashboard/coachhelm/genome/compare-players-players-players', |
There was a problem hiding this comment.
This required route still names /compare-players-players-players. The command palette and App Router page use /golf/dashboard/coachhelm/genome/compare-players, so the manifest assertion can fail even when the palette links to the canonical page.
| '/golf/dashboard/coachhelm/genome/compare-players-players-players', | |
| '/golf/dashboard/coachhelm/genome/compare-players', |
| { route: '/golf/dashboard/qualifiers/create', role: 'coach', category: 'qualifiers', criticality: 'medium' }, | ||
| { route: '/golf/dashboard/stats/team', role: 'coach', category: 'stats', criticality: 'high' }, | ||
| { route: '/golf/dashboard/coachhelm/genome/compare', role: 'coach', category: 'coachhelm', criticality: 'medium' }, | ||
| { route: '/golf/dashboard/coachhelm/genome/compare-players-players-players', role: 'coach', category: 'coachhelm', criticality: 'medium' }, |
There was a problem hiding this comment.
This route table still emits /compare-players-players-players, which has no matching page.tsx and no redirect. Route audit output generated from this table can publish or crawl a dead Genome Compare URL instead of /compare-players.
| { route: '/golf/dashboard/coachhelm/genome/compare-players-players-players', role: 'coach', category: 'coachhelm', criticality: 'medium' }, | |
| { route: '/golf/dashboard/coachhelm/genome/compare-players', role: 'coach', category: 'coachhelm', criticality: 'medium' }, |
| { route: '/golf/dashboard/rounds/new', role: 'player', category: 'rounds', criticality: 'high' }, | ||
| { route: '/golf/dashboard/rounds/recover', role: 'player', category: 'rounds', criticality: 'low' }, | ||
| { route: '/golf/dashboard/rounds/create', role: 'player', category: 'rounds', criticality: 'high' }, | ||
| { route: '/golf/dashboard/rounds/recover-draft-draft', role: 'player', category: 'rounds', criticality: 'low' }, |
There was a problem hiding this comment.
This table still emits /recover-draft-draft, but the offline recovery page is /golf/dashboard/rounds/recover-draft. Consumers of the normalized route list can crawl a non-existent recovery URL and miss the real page.
| { route: '/golf/dashboard/rounds/recover-draft-draft', role: 'player', category: 'rounds', criticality: 'low' }, | |
| { route: '/golf/dashboard/rounds/recover-draft', role: 'player', category: 'rounds', criticality: 'low' }, |
| - src/app/golf/(dashboard)/dashboard/rounds/create/** | ||
| - src/app/golf/(dashboard)/dashboard/rounds/continue/** | ||
| - src/app/golf/(dashboard)/dashboard/rounds/recover/** | ||
| - src/app/golf/(dashboard)/dashboard/rounds/recover-draft-draft/** |
There was a problem hiding this comment.
This registry glob still targets recover-draft-draft/**, while the renamed route directory is recover-draft/**. Feature mapping for round recovery will not match the real recovery page, so registry-based context checks can miss this route.
| - src/app/golf/(dashboard)/dashboard/rounds/recover-draft-draft/** | |
| - src/app/golf/(dashboard)/dashboard/rounds/recover-draft/** |
1 participant
Summary
/new,/recover, and CoachHelm/compareroute paths to canonical/create,/recover-draft, and/compare-playerspaths.Validation
npm run routes:checknpm run test:businessnpm run typecheckgit diff --checkNotes
Greptile Summary
This PR canonicalizes legacy create and recovery route paths across the app. The main changes are:
Confidence Score: 4/5
These route fixes should be corrected before merging.
scripts route tests, route normalizer, and memory registry
Important Files Changed
Reviews (2): Last reviewed commit: "fix(ui): keep button primary classes sta..." | Re-trigger Greptile
Context used (5)