Skip to content

Stop folders named index from collapsing breadcrumb URLs (#542)#694

Merged
srid merged 8 commits intomasterfrom
fix-index-folder-urls
May 2, 2026
Merged

Stop folders named index from collapsing breadcrumb URLs (#542)#694
srid merged 8 commits intomasterfrom
fix-index-folder-urls

Conversation

@srid
Copy link
Copy Markdown
Owner

@srid srid commented May 1, 2026

A note like index/index/index/example.md previously linked its direct-parent breadcrumb to /index/index/ — one folder too shallow. Two trailing-index strips were running back-to-back: Emanote's LML canonicalizer drops foo/index.md's trailing index slug to identify it with foo.md, then Ema's pretty-URL strategy strips the next one again on URL emit. For a folder also named index, that ate a real directory level. Closes #542.

How the fix lines up

The drop and the re-add are now expressed as inverses, side by side in Emanote.Route.R:

Direction Function Behaviour
File path → LML route mkLmlRouteFromFilePath strips trailing index (length > 1)
File path → other route mkRouteFromFilePath unchanged
LML slugs → HTML slugs expandIndexSlug (new) re-adds trailing index (length > 1)
LML route → URL route MR.lmlToHtmlRoute (new) the only sanctioned LML→Html seam

noteHtmlRoute now goes through lmlToHtmlRoute for file-path-derived routes; explicit slug: metadata is taken at face value (the user typed it) and bypasses the expansion.

Refinements during structural review

Hickey and Lowy passes turned up four follow-on findings; each landed as its own commit so the history reads as the diff growing inward:

What was off Fix
Initial draft applied expandIndexSlug to user-supplied slug: metadata too — a slug: foo/index would emit /foo/index/index/, not /foo/index/. Split the slug-metadata branch from the file-path branch in noteHtmlRoute.
The expansion lived inline in noteHtmlRoute, easy for a future HTML-route emitter to skip. Extracted MR.lmlToHtmlRoute as the single named LML→Html seam in ModelRoute.hs.
mkRouteFromFilePath' carried a Bool flag that silently activated the strip rule. Split into mkRouteFromFilePath (no strip) and mkLmlRouteFromFilePath (strips); 3 call sites moved.
Inverse-rule split across mkLmlRouteFromFilePath and expandIndexSlug was discoverable only by hunting. Added a docstring cross-reference at the strip site naming the inverse and the Ema dependency.

Test coverage

  • R.expandIndexSlug × 5 — leaves single slugs / non-index endings alone, appends for the two routetree doesn't like folders named 'index' #542 shapes (foo/index/index.md and index/index/index/example.md).
  • mkLmlRouteFromFilePath × 2 — regression tests for the same shapes, asserting the LML route stays the right length.

Try it locally

nix run github:srid/emanote/fix-index-folder-urls

Behavior change worth flagging: the *.xml URL of a feed-enabled note inside a nested index folder also moves — e.g. foo/index/index.md with feed: enable: true previously served at /foo/index.xml, now at /foo/index/index.xml. The previous URL was a collision with foo/index.md's feed (had it existed), not a feature; users with feeds at nested-index paths may need to update outbound links.

Generated by /do on Claude Code (model claude-opus-4-7).

srid added 6 commits April 30, 2026 22:21
A note like `index/index/index/example.md` rendered its direct-parent
breadcrumb as `/index/index/` (one folder too shallow) because the
folder placeholder for `index/index/index/` encoded to
`index/index/index.html` and Ema's pretty URL stripped the trailing
`index` segment.

The fix mirrors `mkRouteFromFilePath' True`'s "drop trailing index"
rule with an inverse `R.expandIndexSlug`: when an LML route already
ends in `index` (and isn't the lone root), `noteHtmlRoute` re-adds a
trailing `index` slug so the encoded HTML path includes the folder
name and the pretty URL resolves to the correct directory.
The expansion in noteHtmlRoute compensates for `mkRouteFromFilePath' True`
stripping a trailing `index` during file-path parsing. A user-supplied
`slug:` field never went through that strip, so applying the expansion to
it second-guesses the URL the author explicitly typed (e.g. `slug: foo/index`
would emit `/foo/index/index/`, not `/foo/index/`).

Split the two branches: explicit slugs are used as-is, file-path-derived
slugs are expanded.
The trailing-index expansion lived loose in noteHtmlRoute, which made it
easy for a future HTML-route emission site to bypass the rule. Wrap the
LMLRoute → R 'Html conversion in a single named smart constructor in
ModelRoute.hs so the rule has one home and the call site reads as the
intent ("get the HTML route for this LML route") instead of the mechanism
("coerce, unRoute, expandIndexSlug, mkRouteFromSlugs").
Cross-reference expandIndexSlug from mkRouteFromFilePath''s docstring so a
reader hitting the strip rule first can find its inverse without grepping,
and explicitly name Ema's pretty-URL strip as the reason callers need to
re-expand on URL emission.
The two new dropIndex tests for #542 wrote `R $ "foo" :| ["index"]`
inline; without an LML/HTML extension on the right-hand side, GHC
couldn't pin `mkRouteFromFilePath'`'s phantom type and rejected the
test with `Ambiguous type variable 'ext1'`. Use the existing `r1Index`
named route for the two-slug case and add `rNested` for the four-slug
case so the type flows from the test fixture.
The Bool flag obscured the LML canonicalization rule — a `True` argument
silently activated the trailing-`index` strip. Replace `mkRouteFromFilePath' True`
with a dedicated `mkLmlRouteFromFilePath` whose name carries the rule, and
fold the `False` body into `mkRouteFromFilePath`. Three call sites move
(ModelRoute, Source/Patch, RSpec); the public surface is now two
self-explaining functions instead of one with a magic flag.
@srid
Copy link
Copy Markdown
Owner Author

srid commented May 1, 2026

Hickey/Lowy Analysis

# Lens Finding Disposition
1 Hickey expandIndexSlug applied to user-supplied slug: metadata, producing an unexpected double-index slug Fixed in this PR
2 Hickey Index-slug rule split across mkRouteFromFilePath' and expandIndexSlug with no enforced coupling Fixed in this PR (lmlToHtmlRoute smart constructor)
3 Hickey No static guarantee that all HTML-route sites call expandIndexSlug Fixed in this PR (subsumed by #2)
4 Hickey noteHtmlRoute index-key change is correct but creates an invisible precondition for R 'Html lookups No-op (by design — one consistent invariant)
5 Lowy expandIndexSlug duplicates index-stripping rule without naming the shared axis Fixed in this PR (cross-references in both docstrings)
6 Lowy Ema-specific compensation unnamed at the call site Fixed in this PR (smart-constructor doc names Ema)
7 Lowy Ema integration assumption belongs in a dedicated module No-op (expandIndexSlug is genuinely about route encoding, not Ema-specific glue; a dedicated module would over-encapsulate)
8 Lowy Boolean flag mkRouteFromFilePath' True obscures the LML canonicalization rule Fixed in this PR (split into mkRouteFromFilePath / mkLmlRouteFromFilePath)
9 Lowy noteHtmlRoute dual use (URL emit + IxNote index key) now encodes Ema's behavior in the index No-op (subsumed by Hickey #4 — one canonical HTML route legitimately serves both)

Hickey rationale

The original diff fragmented the index-slug invariant: mkRouteFromFilePath' True strips on the way in, expandIndexSlug re-adds on the way out, and nothing in the type system enforces that an HTML-route emitter calls the latter. The fix is twofold: (a) the slug-metadata branch of noteHtmlRoute was second-guessing an explicitly-typed URL — split out so the expansion only fires on file-path-derived routes; (b) the expansion now lives inside MR.lmlToHtmlRoute, the named LML→Html seam, so the call site reads as intent ("get the HTML route for this LML route") instead of mechanism. Future HTML-route emitters that go through this seam can't forget the rule. The IxNote index-key change is part of the same canonical-route invariant — the lookup uses what the encoder produces, end of story.

Lowy rationale

The genuine volatility here isn't "should we strip?" — that's a stable design decision — but "what does Ema's pretty-URL strip do?" The fix encapsulates the Ema-strip dependency in expandIndexSlug and names it explicitly in three doc comments (the helper, the strip site, the smart constructor). A dedicated Integration/Ema.hs module would over-encapsulate for a single boolean of behavior; the cross-references are sufficient. The boolean-flag refactor was orthogonal to the bug but small enough to land cleanly — the public surface is now two self-explaining functions. The noteHtmlRoute dual use stays one boundary because both callers (URL emit + IxNote index) consume the same canonical route; splitting them would invite drift.

@srid
Copy link
Copy Markdown
Owner Author

srid commented May 1, 2026

Evidence

End-to-end visual proof of the fix for #542 against a notebook containing index/index/index/example.md. The parent breadcrumb / sidebar link previously rendered as /index/index/ (one folder too shallow) and now correctly renders as /index/index/index/.

Before (master) After (this PR)
Before — broken parent link After — correct parent link
Parent breadcrumb href resolves to /index/index — only two index segments, missing one. Parent breadcrumb href resolves to /index/index/index — all three index segments preserved.

Verified by reading document.querySelectorAll('a[href]') on each page; the offending href moves from index/index to index/index/index exactly where #542 predicted.

@srid
Copy link
Copy Markdown
Owner Author

srid commented May 1, 2026

/do results

Step Status Duration Verification
sync 1s forge=github; noGit=false
research 20m 29s Identified bug in noteHtmlRoute: routes ending in index get pretty-URL-stripped a second time, collapsing nested-index folders by one level.
branch 4s Created fix-index-folder-urls from origin/master
implement 3m 43s Added R.expandIndexSlug helper + tests + CHANGELOG entry
check 42s cabal build all clean
docs 28s CHANGELOG.md updated; existing folder-note.md unchanged (routing fix, no doc rewrite needed)
fmt 25s cabal-fmt, fourmolu, hlint, nixpkgs-fmt all pass
commit 32s Primary feature commit pushed
hickey+lowy 8m 27s 3 follow-up commits: split slug-metadata branch, introduced lmlToHtmlRoute smart constructor, doc cross-references
police 1m 29s All 3 passes clean (rules / fact-check / elegance)
test 1m 20s 56/56 unit tests pass; one type-ambiguity fix landed mid-loop
create-pr 5m 50s PR #694 created; Hickey/Lowy analysis posted with 9-finding ledger
ci 2m 27s vira ci signed off on b0b1baae; e2e-live 29/29, e2e-static 27/29 (2 mode-skipped), e2e-morph 29/29
evidence 3m 31s Before/after screenshots posted; href moved from /index/index to /index/index/index
Total 50m 4s

Slowest step: research (20m 29s)

Optimization suggestions

  • Front-load the research-phase Explore subagent — research dominated the wall-clock (40% of total). The first Explore subagent returned a strong file:line map, but a chunk of time then went into manually re-walking the same code paths in the main context to re-derive the Ema-strip / LMLRoute-strip composition. A second targeted Explore (specifically: "trace routeUrlWith UrlPretty end-to-end on a stripped LMLRoute") would have saved several main-context reads.
  • Hickey/Lowy on a smaller initial diff — the structural review (8m 27s) drove 4 follow-up commits because the initial diff applied expandIndexSlug too broadly (slug-metadata branch). Splitting branches up front in noteHtmlRoute would have removed the largest finding before review even ran.
  • Tests caught a type-ambiguity that cabal build of the library missedcabal build all builds the test executable but doesn't compile-check unit tests inside it until cabal test all. Running cabal test all --test-options=--no-create (or cabal build all --enable-tests) during the check step would surface test compile errors a step earlier.
  • e2e suite wall-clock — e2e-live / e2e-static / e2e-morph took ~3-10s each but each pre-build emanote separately. Running them in parallel (which this run did via run_in_background) is the right call — keep doing that on --from ci-only re-runs.

Workflow completed at 2026-04-30 22:48 UTC.

@srid
Copy link
Copy Markdown
Owner Author

srid commented May 1, 2026

Re-captured evidence

The previous shot pair looked identical at a glance because the page bodies are the same — only the colored top banner was differing. This re-capture navigates to the immediate-parent breadcrumb on each build, so the load-bearing evidence is the post-click URL.

Before (master) After (PR #694)
before after
URL after click: /index/index (one folder too shallow — bug) URL after click: /index/index/index (correct immediate parent)

Starting from /index/index/index/example and clicking the parent-breadcrumb link: master lands on /index/index/ (the grandparent), PR #694 correctly lands on /index/index/index/. The placeholder pages render identically by design, so the URL is the only visible difference; banner echoes location.pathname so the screenshot captures it without browser chrome.

Add a fixture at `index/index/index/example.md` and a Cucumber
scenario that opens the note and asserts the immediate-parent
breadcrumb href contains `index/index/index` (positive) and is not
the buggy `index/index` value (negative). Verified that reverting
`noteHtmlRoute` to master's body makes the new scenario fail with
the expected diagnostic. Passes in all three modes (live / static /
morph).
@srid
Copy link
Copy Markdown
Owner Author

srid commented May 1, 2026

E2E coverage added (commit 23d0eb7)

Cucumber scenario in tests/features/smoke.feature opens the bug-trigger fixture and asserts the immediate-parent breadcrumb's href:

Scenario: Folders named index don't collapse breadcrumb URLs (regression: #542)
  When I open "/index/index/index/example.html"
  Then the immediate-parent breadcrumb href contains "index/index/index"
  And the immediate-parent breadcrumb href does not equal "index/index"

Verified end-to-end:

  • Passes with the fix in place (live + static + morph): 30/30, 28/30 mode-skipped, 30/30.
  • Fails when noteHtmlRoute is reverted to master's body, with the expected diagnostic: Immediate-parent breadcrumb href expected to contain "index/index/index", got "index/index". The trailing-index expansion regressed (R.expandIndexSlug or MR.lmlToHtmlRoute), so Ema's URL strip ate one folder level — see #542.

Fixture is tests/fixtures/notebook/index/index/index/example.md — three folders deep, all named index.

Pre-#542 the routes for `foo/index.md` (folder note for `foo/`) and
`foo/index/index.md` (folder note for the deeper `foo/index/`)
collapsed onto one HTML route — the IxNote dedup picked one and the
other became unreachable. Lock the post-fix behaviour from both ends:

* Six unit cases in `RSpec.hs` (`folderNoteCoexistenceSpec`) pin
  every step from file path to HTML route slugs and assert the three
  shapes — folder note, nested-index folder note, sibling file —
  produce three distinct routes.

* One Cucumber scenario (`smoke.feature`) and a depth-indexed step
  (`smoke_steps.ts`) open `/subfolder/index/example.html` and assert
  the depth-1 breadcrumb (folder note `subfolder.html`) and the
  depth-2 breadcrumb (placeholder `subfolder/index`) carry distinct
  hrefs containing `subfolder` and `subfolder/index` respectively.

Verified to fail on master's `noteHtmlRoute` body with the expected
diagnostic ("Breadcrumb at depth 2 expected href to contain
'subfolder/index', got 'subfolder'") before restoring the fix.
Passes in all three modes.
@srid
Copy link
Copy Markdown
Owner Author

srid commented May 2, 2026

Coexistence coverage added (commit d842cc5)

Per review request, locking down the case where foo/index.md (folder note for foo/) coexists with a same-named child folder foo/index/{index.md, bar.md, …}. Pre-#542 these collapsed onto one HTML route and the IxNote dedup made one of them unreachable; the fix sends them to distinct routes, and these tests pin that.

Unit suitefolderNoteCoexistenceSpec in RSpec.hs (6 cases):

File path LML route Expanded HTML route
foo/index.md R("foo") R("foo") (length 1, no expansion)
foo/index/index.md R("foo","index") R("foo","index","index") (#542 expansion)
foo/index/bar.md R("foo","index","bar") R("foo","index","bar") (last slug not "index")

The suite asserts each step is exact and that all three shapes produce distinct HTML routes — a regression that re-collides them surfaces here, not as a runtime "ambiguous notes" error.

E2E scenarioFolder note coexists with a same-named child folder (regression: #542) in smoke.feature:

When I open "/subfolder/index/example.html"
Then the breadcrumb at depth 1 has href containing "subfolder"
And the breadcrumb at depth 2 has href containing "subfolder/index"
And the breadcrumb at depth 1 has a different href from depth 2

Fixture tests/fixtures/notebook/subfolder/index/example.md lives alongside the existing subfolder/index.md (folder note exercised by the #608 scenario). Three modes pass: live 31/31, static 29/31 (2 mode-skipped), morph 31/31. Verified to fail on master with Breadcrumb at depth 2 expected href to contain "subfolder/index", got "subfolder".

@srid srid marked this pull request as ready for review May 2, 2026 12:52
@srid srid merged commit d98c9ab into master May 2, 2026
4 checks passed
@srid srid deleted the fix-index-folder-urls branch May 2, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

routetree doesn't like folders named 'index'

1 participant