Skip to content

fix: The new-term composer's breadcrumb still folds the letter off the slug and links it home - #9628

Open
usirin wants to merge 1 commit into
mainfrom
build/9602-composer-crumb-letter-802b0e8f
Open

usirin wants to merge 1 commit into
mainfrom
build/9602-composer-crumb-letter-802b0e8f

Conversation

@usirin

@usirin usirin commented Sep 21, 2026

Copy link
Copy Markdown
Member

The sözlük term page drew its breadcrumb twice: SozlukTermHeader for a term that exists, and a
second copy inlined in NewTermComposer for a slug nobody has defined yet. The two drifted —
PR #9600 (#9355) and the fold fix (#9331) each landed on the header alone, so the composer was
still pointing its letter crumb at /sozluk and folding the letter with
slug.charAt(0).toLowerCase().

Which file owns the crumb markup now

apps/web/src/components/sozluk/SozlukTermCrumbs.tsx — new, and the only renderer of
.kp-sozluk-term__crumbs on this page. Both branches render it:

  • apps/web/src/components/sozluk/SozlukTermHeader.tsx passes the stored first_letter column.
  • apps/web/src/pages/SozlukTermPage.tsx (NewTermComposer) passes sozlukLetterOf(slug) from
    apps/web/worker/features/sozluk/turkish-alphabet.ts — the term does not exist yet, so there is
    no column to read, and that leaf module is the fold every producer of the column shares.

The component builds the target through sozlukLetterHref(letter, false) and renders no letter
crumb when the letter is empty or null. So there is one derivation site per branch feeding one
markup, and a fourth crumb fix cannot land on only one view.

Both crumbs are router Links now; the composer's were raw <a href>, a full page load on a
route the SPA already owns.

Tests

apps/web/src/pages/SozlukTermPage.test.tsx gains a NewTermComposer breadcrumb block covering
the Turkish pairs — IŞIK and ışık both resolve to /sozluk/harf/%C4%B1, İMECE and imece
both to /sozluk/harf/i — plus the unindexed slug (3-adim renders one crumb link, the root) and
the two-Link routing assertion. SozlukTermHeader.test.tsx is unchanged and still passes through
the shared component. 17/17 green in apps/web project client for the two touched files.
fabrika build check --surface code is green.

Deviations

  • Said: attach before/after captures of the rendered surface.
    Did: shipped with no captures; the DOM assertions in the tests stand in.
    Why: fabrika ui render --surface /sozluk/<slug> exits 11 in this worktree — app "web" did not answer 200 on /api/health within the readiness bound — every surface is UNKNOWN, the worker backend never binding (ECONNREFUSED on the vite proxy). Reproduced twice. That is the known worktree render gap tracked in A fresh worktree cannot render the web surface: .env.example has no Cloudflare credentials #9497, so the before state was never capturable either.
    Disposition: the PR preview is the rendering surface for this change; the render gap is on the record for the design gate to judge.

  • Said: the ticket offers a shared breadcrumb component or a narrower in-place fix.
    Did: took the shared-component route.
    Why: acceptance criterion 6 asks that the two views stop carrying independent letter derivations, and the in-place fix would leave two copies of the markup that already drifted twice.
    Disposition: intended; SozlukTermCrumbs.tsx is the one owner, named above.

  • Said: keep the diff to this ticket's surface.
    Did: exported NewTermComposer from SozlukTermPage.tsx.
    Why: the crumb is asserted against the real component rather than a re-implementation of its derivation.
    Disposition: intended; no other call site changes.

  • Said: build to the repo's design law.
    Did: LAW-SOURCE: manifest-prose.
    Why: fabrika ui manifest reports no typed prohibition registry ("registry": null).
    Disposition: no new tokens, values or styles in this diff; markup, class names and CSS are unchanged, so no visual change is intended. Crumb link styling belongs to Sozluk term crumbs read as plain text, letter-page crumbs as links #9608, and SozlukTermPage.css and the subnav chrome are left alone so this does not collide with fix: /pano overflows the mobile viewport: 500px of document in a 390px window #9586.

Fixes #9602

🤖 Generated with Claude Code

The term page drew its breadcrumb twice — once in `SozlukTermHeader`, once inline in
`NewTermComposer` — and the two copies drifted. #9355 and #9331 each landed on the header
alone, leaving the composer's letter crumb pointing at `/sozluk` (where the root crumb beside
it already goes) and naming the letter with `slug.charAt(0).toLowerCase()`, which reads `IŞIK`
as `i` when that headword is `ı`'s everywhere else.

`SozlukTermCrumbs` now owns the markup for both branches and takes the letter as a prop: the
stored `first_letter` column from the header, `sozlukLetterOf(slug)` from the composer, which
is the same Turkish-aware fold every producer of that column shares. A slug the alphabet does
not index yields `null` and renders no letter crumb, matching the header. Both crumbs are
router `Link`s, so a click no longer reloads a route the SPA owns.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Preview deployed

@usirin

usirin commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ d767a0a content:9667aae9291e — merge-ready

Round 1, code class. Namespaces this head derives: review-code (mine) and review-ui (routed to
the design gate — not judged here, and its absence below is not a gap in this verdict).
CI at head: settled / green, 47 check runs, 28 of 43 repo-authored workflows inspected this
commit. Typecheck, lint and unit tests are not re-run here.

Per-criterion verification

  • [PASS] On /sozluk/<slug> for an undefined slug, the letter crumb links to
    /sozluk/harf/<letter>, built through sozlukLetterHref, not to /sozluk.

    SozlukTermCrumbs.tsx:28 builds the target as sozlukLetterHref(letter, false); the false
    is right, because sozlukLetterHref returns bare /sozluk only on the active-letter arm and
    the term page has no active letter. SozlukTermPage.tsx:211 renders that component in
    NewTermComposer, replacing the two <a href="/sozluk"> crumbs. Asserted in
    SozlukTermPage.test.tsx/sozluk/harf/i for imece, /sozluk/harf/%C4%B1 for IŞIK.

  • [PASS] The letter is derived through sozlukLetterOf (or the same foldTurkishChar fold),
    never toLowerCase().

    SozlukTermPage.tsx:207const letter = sozlukLetterOf(slug), imported from
    apps/web/worker/features/sozluk/turkish-alphabet.ts. Read that module at this head:
    sozlukLetterOf folds through foldTurkishChar, whose UPPER_TO_LETTER map carries the
    I → ı / İ → i pair before any ASCII lowering, and it is the same function
    storedFirstLetter wraps for the first_letter column's producers. So the composer's letter
    and the header's column now come off one fold. slug.charAt(0).toLowerCase() is gone.
    The leaf import into the SPA bundle is the established route, not a new edge: that module
    declares itself dependency-free for exactly this, and src/components/sozluk/Sozluk.tsx and
    src/lib/sozlukLetterParam.ts already import it.

  • [PASS] A slug whose first character falls outside the sözlük alphabet renders no letter
    crumb, matching SozlukTermHeader.

    sozlukLetterOf answers null for an unindexed first character, and SozlukTermCrumbs.tsx:26
    guards on letter ? — so null renders root + title and nothing between. The header's arm
    matches through the same guard: it passes the stored column, which is "" for an unindexed
    headword (storedFirstLetter), and "" is falsy on that same branch. One guard, both
    branches. Covered by the 3-adim case, which asserts exactly one anchor.

  • [PASS] Both composer crumbs are router Links, so a crumb click does not reload the page.
    SozlukTermCrumbs.tsx lines 1, 25 and 28 — both crumbs are Link from react-router, and the
    composer's previous raw <a href> pair is deleted in SozlukTermPage.tsx. Discharged by the
    code read. One non-blocking note on the accompanying test, below.

  • [PASS] A test covers the Turkish pairs: a slug starting I/ı resolves to ı, and one
    starting İ/i resolves to i.

    Four cases in the new NewTermComposer's breadcrumb letter block: IŞIK and ışık both to
    /sozluk/harf/%C4%B1, İMECE and imece both to /sozluk/harf/i. The IŞIK case also
    asserts no i link exists, which is the assertion that would have caught the old fold — the
    bug produced an i crumb, not a missing one. The tests drive the real exported
    NewTermComposer rather than a re-implementation of its derivation, so they are honest about
    what they cover.

  • [PASS] The composer's crumb and the header's crumb no longer carry two independent letter
    derivations.
    [evidence: the PR description naming which file now owns the crumb markup]
    Graded on that evidence, read in the PR body: the section ## Which file owns the crumb markup now names apps/web/src/components/sozluk/SozlukTermCrumbs.tsx as the new and only renderer
    of .kp-sozluk-term__crumbs on this page, and names both callers — SozlukTermHeader.tsx
    passing the stored first_letter, SozlukTermPage.tsx passing sozlukLetterOf(slug). The
    diff agrees with that claim rather than merely restating it: the header's inline
    <p className="kp-sozluk-term__crumbs"> block is deleted and its Link and sozlukLetterHref
    imports go with it, the composer's inline block is deleted, and .kp-sozluk-term__crumbs
    appears exactly once at this head — in the new component. Two sources feeding one markup, with
    no fold of its own in the component, is the shape the criterion asks for.

Standing checks

  • Test honesty. No pre-existing test or assertion is changed or deleted; the only edit to the
    test file outside the new block is the added NewTermComposer import.
    SozlukTermHeader.test.tsx is untouched and still exercises the header branch through the
    extracted component.
  • Release containment. No new surface. This changes an existing crumb's href and letter on a
    page that already rendered both crumbs; no containment is owed.
  • Comment discipline. The three comments earn their place: the props docblock states the
    invariant the types cannot (callers supply the letter, the component never folds), the
    NewTermComposer comment states why the letter comes off the slug rather than a column, and
    the header's existing fold comment is left in place. The component docblock's second half is
    drift history rather than a constraint; it is two lines and it is what makes the extraction
    legible at the call site, so it is not raised as a finding.
  • Staleness traps. None — nothing is cached across a boundary that moves.
  • Behaviour claims. The one runtime claim this verdict rests on — that sozlukLetterOf folds
    I to ı — is read out of turkish-alphabet.ts at this head, not inferred from the name.

Non-blocking notes (no criterion turns on either; neither routed to the contract)

  1. The routes both crumbs through the router test asserts only the two href values, which a
    pair of plain <a> tags would also satisfy — so it locks the targets, not the routing. The
    criterion is discharged by the code, and hardening that assertion would be a nicety, not a
    repair. Left as a note deliberately: appending it would fail this round over a test that
    already covers the bug.
  2. The breadcrumb is a <p> with slash separators and no nav / aria-label / aria-current.
    That is pre-existing markup this PR moved verbatim, out of scope here, and filed separately
    rather than held against this diff.

Deviations

# Disclosed Substance check
1 No before/after captures; fabrika ui render exits 11 in the worktree (#9497) Accurate, and matches the known worktree render gap. The rendered judgment is review-ui's; this entry hands it the fact rather than hiding it.
2 Took the shared-component route over the narrower in-place fix Matches the diff, and is what criterion 6 asks for.
3 Exported NewTermComposer so the test drives the real component Matches the diff. It widens the module's surface by one symbol with no other call site, and the alternative — asserting a re-implementation — would have been the class-6 problem.
4 LAW-SOURCE: manifest-prose; no new tokens, values or styles Matches the diff: no CSS file is touched and every class name is carried over unchanged.

Nothing undisclosed that this gate could see; the section is present and none of it is falsified.

review-code: PASS.

Verdict-written: 2026-09-21T04:49:38Z

@usirin

usirin commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

review-ui: CANT-SEE at d767a0a

No verdict posted. The one state this PR changes for a reader never painted, so a PASS here would
be a judgment I did not form. Cause: no-preview-render.

What I could render

Preview phoenix-phoenix-pr-9628-...workers.dev, at head d767a0ad, anonymous, desktop 1280x800
and mobile 390x844. Nine captures, zero page errors.

Surface Viewports What painted
/sozluk/ISIKTEST9602 desktop, mobile NotFoundPage — "terim bulunamadı", not the composer
/sozluk/imecetest9602 desktop, mobile same
/sozluk/3-adim-test9602 desktop, mobile same
/sozluk desktop index, for slug discovery
/sozluk/isik desktop the existing-term branch of the extracted component

What I could not render, and why

NewTermComposer — the whole subject of #9602 — renders only for a signed-in reader.
SozlukTermContent returns NotFoundPage when !term && !signedIn, so every anonymous shot of an
undefined slug is the not-found page. That is a clean capture of a composition this PR did not
touch: the crumb fix is not in it.

The :auth render refused on 11 for all six shots:

surface "/sozluk/ISIKTEST9602:auth" at desktop did not render signed in (the preview answered the
seeded cookie as a visitor) — the authenticated render is UNKNOWN, never the anonymous one.

The refusal is post-signing, so the failure is on the preview side of the cookie, not an unset
operand: either this PR's preview D1 was never seeded with the yazar test account and its session
row, or the signing secret I resolved is not the one this preview worker deploys with. Both are
operator acts. Seeding the preview D1 with preview-seed test-account, or handing the gate the
ci-credentials export for --auth-secret-from, unblocks the re-run.

The half that did paint, and what it shows

/sozluk/isik renders the header branch through the new SozlukTermCrumbs: the crumb reads
sözlük / ı / ışık — the dotless ı, correctly folded, with the letter as its own link. The
extraction did not regress the branch it moved out of, at either width. Nothing else in the diff
changes paint: no CSS, no class names, no tokens. Advisory only — it is not the criteria's surface.

Deterministic tier

Not recomputed. heal-ci surface at this head shows four required contexts producing, and the
design-adjacent guards (property-based a11y over @kampus/design, check every component CSS file consumes the design-token seam) running as extra. This diff adds no CSS and no tokens, so I
expect no movement from either.

Not this PR's

Crumb link styling is #9608's. Subnav.css / stickyChrome is #9586's. Neither is grounds here,
and I raise neither.

@usirin

usirin commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

The rendered-visual gate could not see this PR's surface at d767a0ad. Second round, same cause
(no-preview-render), but narrower than last time.

The surface this PR fixes is NewTermComposer, and SozlukTermPage.tsx gates it on
useSession() — an anonymous reader of an undefined slug gets NotFoundPage, so an anonymous
capture cannot carry this judgment. The tier render is the only way in, and it refused again.

Exact refusal, exit 11:

review-ui render: surface "/sozluk/3-adim:auth" at desktop did not render signed in (the preview answered the seeded cookie as a visitor) — the authenticated render is UNKNOWN, never the anonymous one.

What I ruled out this round, so the next operator does not re-seed for nothing:

  • The preview announces D1 8f426d25-62ee-4e1e-8e4c-b9cec2a0fd82, and that database holds both
    seeded session rows — preview-test-moderator and preview-test-caylak — each with a 64-char
    token and an expiry of 2026-09-28. The seed landed on the database this preview actually binds.
  • All three credentials resolve in the render environment, none empty, none carrying the
    insecure_ placeholder, and both tier tokens are 64 characters, the same length as the stored
    rows.
  • The refusal is the no-user branch, not a tier mismatch, so it is not a wrong-audience shot.

That leaves one unverified piece: the BETTER_AUTH_SECRET the render signs the cookie with is not
provably the one this preview worker deploys with. A well-formed cookie under the wrong secret is
answered as a visitor, which is exactly the observed state. Its one readable copy is the
ci-credentials stack's alchemy state behind $ALCHEMY_PASSWORD, and that variable is unset here,
so I cannot export it and pass --auth-secret-from. An operator holding it is the way through.

No verdict posted, and none is inferable: the composer's crumb never painted, so this ticket's
acceptance criteria are UNKNOWN rather than met. /sozluk/cagri did render, showing the header
branch's crumb reading sözlük / ç / çağrı, but that is the half of the diff this ticket was not
filed about, and it is not a recorded capture — the run exited non-zero, so nothing new entered
the set manifest.

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.

The new-term composer's breadcrumb still folds the letter off the slug and links it home

1 participant