fix(resolver): return not-found for unmatched path segments

[sv2dev]

Summary

• Fix path segment skipping bug: When a path segment didn't match any route (no exact match, wildcard, or virtual fallback), the resolver silently skipped it and moved to the next segment. This caused /x/y to incorrectly resolve the /y handler when no /x route existed.
• Remove duplicate guard invocation: Guards were called twice per level during path traversal — once via checkGuard() and again by an inline duplicate check.
• Add 13 new tests covering not-found scenarios, guard behavior, and wildcard matching (55 total).

Test plan

• Existing tests still pass
• New "not found" tests verify /x/y doesn't resolve when only /y exists (and vice versa)
• New tests verify unmatched intermediate segments return not-found
• New tests verify guards are called exactly once per level
• New tests verify wildcard param collection and exact-match priority

Summary by CodeRabbit

• Bug Fixes

  • Improved route resolution with better handling of unmatched segments and early termination.
  • Enhanced guard behavior to execute once per level and properly short-circuit on redirects.

• New Features

  • Added support for improved wildcard route matching and parameter collection.
  • Better not-found detection for missing path segments and unmatched routes.

[coderabbitai]

Walkthrough

Route resolution logic was simplified by removing optional guard special handling and adding early termination for unmatched segments. Test coverage was expanded to include not-found scenarios, guard behavior across nested levels, and wildcard parameter matching.

Changes

Cohort / File(s)	Summary
Test Expansion packages/esroute/src/route-resolver.spec.ts	Restructured test suite with new describe blocks for "not found" (unmatched paths), expanded "guards" (guard invocation frequency and short-circuiting), and "wildcards" (multiple wildcard params, exact match preference). Removed original minimal guard test coverage.
Resolution Logic Simplification packages/esroute/src/route-resolver.ts	Removed special-case handling for optional routes["?"] guards; added explicit else fallback to return null immediately for unmatched segments when no routes, wildcards, or virtual-object handlers apply.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A route found its path, more clear than before,
Guards guard just once now, no more and no more,
Wildcards dance free, while unmatched paths fall,
The tests now run deep, catching edge cases all! 🌿✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely describes the main fix—returning not-found for unmatched path segments. This aligns perfectly with the PR's primary objective of fixing a path-segment skipping bug in the resolver.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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 fix/route-resolution-skip-unmatched-segments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/esroute/src/route-resolver.spec.ts (1)

7-7: ⚠️ Potential issue | 🟡 Minor

Shared mock notFound is never reset, causing potential test pollution.

The notFound mock is created once at the describe block level and reused across all tests without being reset. Tests asserting expect(notFound).toHaveBeenCalled() (lines 117, 127, 137, 145, 155) will pass if notFound was called in any previous test, masking actual failures.

Proposed fix: Reset mock between tests

 describe("Resolver", () => {
   const notFound = vi.fn();
+
+  beforeEach(() => {
+    notFound.mockClear();
+  });

Alternatively, add vi.clearAllMocks() in a beforeEach to reset all mocks.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/esroute/src/route-resolver.spec.ts` at line 7, The shared mock
notFound defined at the describe scope is never reset and can cause test
pollution; update tests to clear or reset mocks between tests by adding a
beforeEach that calls vi.clearAllMocks() or notFound.mockClear() (or
notFound.mockReset()) so assertions like expect(notFound).toHaveBeenCalled() in
the individual tests only reflect that test's calls; locate the notFound mock
and add the beforeEach reset near the describe block to ensure isolation.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/esroute/src/route-resolver.spec.ts`:
- Line 7: The shared mock notFound defined at the describe scope is never reset
and can cause test pollution; update tests to clear or reset mocks between tests
by adding a beforeEach that calls vi.clearAllMocks() or notFound.mockClear() (or
notFound.mockReset()) so assertions like expect(notFound).toHaveBeenCalled() in
the individual tests only reflect that test's calls; locate the notFound mock
and add the beforeEach reset near the describe block to ensure isolation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7646c4df-34d0-4c0f-bd41-821443cb29f6

📥 Commits

Reviewing files that changed from the base of the PR and between 1706d47 and 7cd3308.

📒 Files selected for processing (2)

• packages/esroute/src/route-resolver.spec.ts
• packages/esroute/src/route-resolver.ts