fix: reading line is a conversation, not a lesson (translations click/hover-only)#48
Conversation
The reading line teaches nothing inline: no inline romanization, no inline gloss, no "means: X". A word's romanized pronunciation and meaning are revealed only on demand in the click word-card popover. Removes the `token-translit` inline sub-label from the Tokens spec, adds the romanized pronunciation to the word-card, and adds the "Reading pedagogy — the binding law" subsection as the governing rule the ar/zh corpora and renderers follow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… card Undoes the inline transliteration sub-label that violated the reading-pedagogy binding law (DESIGN.md §Components): ArWordToken no longer stacks a Latin romanization beneath a new/learning Arabic word — the token renders script only (its `<bdi>` with ḥarakāt fade + status wash + click), same as before the sub-label landed. The romanized pronunciation now surfaces only on click, in a new `data-breakdown-pronunciation` row of ArBreakdownPanel (root · pattern · pronunciation · gloss), styled with the `transliteration` type tokens (--la-font-mono / --la-type-annotation-size / --la-color-muted-soft) and omitted when the token carries no `translit`. The `translit` annotation field is unchanged — it now feeds the card instead of the line. RED withholding, the `<bdi dir="auto">` isolate + ambiguous-first fallback, and the ḥarakāt fade are untouched. Unit test flipped to assert no inline romanization for any status and that the panel shows the pronunciation row when opened. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lation) The demo fixtures spelled a target word's meaning out inline — "the neighborhood market" after سُوق, and "— that means: can." after 可以 — which the reading-pedagogy binding law forbids (the reading line never translates a target word). Rewrites both English scaffolds into natural, pure-context prose where the surrounding sentence carries the sense: the learner meets سُوق / قَهْوَة / مَدِينَة inside a slow-morning scene, and 可以 inside "take the window seat if you'd like." The Arabic and Chinese word tokens and every one of their gate-verified annotations (diac/skeleton/vowelState/morphology/translit/gloss; perCharacter/citationPinyin/surfacePinyin/etc.) are kept exactly — only the surrounding English `text` tokens change, and the meaning-stating tokens are dropped. ar blend.ratio stays 0.2 (arWords 3/12 = 0.25, under the fixture test's 0.34 ceiling). Fixture-test comment reworded: the translit annotation now feeds the click card, not an inline sub-label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
julianken-bot
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Design-correctness fix: the /ar reading line stops teaching inline; romanization moves to the click card. The central risk — does any inline romanization or meaning-gloss survive in the reading line or fixtures? — verified NO, at the code, fixture, test, and doc level.
Verification ledger (all run this turn, worktree @ 4e69ec3)
pnpm install --frozen-lockfile→ OK;pnpm run build→ 8/8 green.pnpm run test→ all green: lang-ar 47, lang-zh 64, lang-fake 66, lang-conformance 20, web 39, server 54 (+9 skipped).pnpm run boundary:renderer-neutrality→ PASS (both gates, exit 0).pnpm run spec:generate:node+git diff→ar-annotations.schema.jsonCLEAN (no drift;translitfield retained, now feeds the card).- Mermaid render check → 1 block,
ok: true(R15 pass). ArWordToken.tsxinline render:<bdi>script only + click-panel. Grep fordata-translit/lang="ar-Latn"/transliterationStylein the token/shell → none in any inline render path; the onlyar-Latnin a render position is the card's new pronunciation row. Tests assert its inline absence for every status (new/learning/known/RED).ArBreakdownPanel.tsx: gains adata-breakdown-pronunciationrow (root · pattern · pronunciation · gloss);data-breakdown-glossrow intact. Schema:morphologyis required,translitoptional, and both come from the same parsedann— so the pronunciation row's nesting under{morphology ? …}has no "translit-without-morphology" hole.- Fixtures: Arabic word tokens in
ar-blend-intro.jsonand the zh可以token inzh-mixed-line.jsonare byte-identical to base (only English scaffoldtexttokens changed; the meaning-stating tokens dropped). ar fixture still validates againstArAnnotationsSchema(server test green). - Base-direction (LTR gentle blend):
ar-message-container.test.tsxuntouched by this PR — the correct feature is preserved. - Doc-currency:
DESIGN.mdcarries the new binding law; grep confirms no surviving "in-text romanization ships" /token-translitsub-label claim (the line-9/128 "training wheels" is the FADE metaphor, not romanization).tokens.jsonuntouched (reuses--la-font-mono/--la-type-annotation-size/--la-color-muted-soft).AGENTS.md/CLAUDE.mdnot touched (shim check N/A).No Figma update neededis coherent (frame77:2already shows the card-reveal model).
Findings
SUGGESTION — no rendered before/after screenshot for a UI-visible change. The PR touches UI source (ArWordToken.tsx, ArBreakdownPanel.tsx) and, beyond removing the inline sub-label, adds a new visible surface (the card's pronunciation row) — so "removal-only UI change" in the body slightly undersells it. The body carries DOM-text evidence + a clean-console report but no user-attachments image. Given the change is removal-dominant, the panel-open path is covered by renderer.spec.ts, and the structure/token-binding is unit-pinned, this is polish, not a blocker — but a before/after of (a) the script-only reading line and (b) the click card's new pronunciation row would harden the per-HEAD screenshot record and visually confirm RTL/ḥarakāt still render. Not merge-blocking.
Bottom line
The build obeys the binding law: reading line = script only, romanization = card-only, gate-verified annotations preserved, base-direction feature intact, docs reconciled, all gates green. Clean APPROVE.
— @julianken-bot (opus). Same-tier note: implementer was Claude Opus 4.8 (same tier as this reviewer); I ran the full build/test/boundary/spec suite myself rather than trusting the reported "all green," and did a mandatory second pass — the strongest candidate defect (pronunciation row gated behind morphology) dissolved under the schema's required-morphology invariant.
|
@Mergifyio queue |
|
Queued — the merge queue status continues in this comment ↓. |
Merge Queue Status
This pull request spent 8 seconds in the queue, including 1 second running CI. Required conditions to merge
|
Diagrams
The reading line stops teaching inline; the romanization moves to the click card.
flowchart LR subgraph Before["Before — violates the law"] b_word["Arabic word سُوق"] --> b_line["reading line: script + INLINE romanization sūq"] b_word -.click.-> b_card["word card: meaning only"] b_fix["fixture scaffold: 'the neighborhood market', '— that means: can.'"] --> b_line end subgraph After["After — conversation, not lesson"] a_word["Arabic word سُوق"] --> a_line["reading line: SCRIPT ONLY (wash + ḥarakāt fade)"] a_word -- click/hover --> a_card["word card: root · pattern · PRONUNCIATION sūq · gloss"] a_fix["fixture scaffold: pure-context prose, no translation"] --> a_line end Before ==>|fix| AfterSummary
"the neighborhood market","— that means: can."). Both are undone.ArBreakdownPanelgains apronunciationrow (root · pattern · pronunciation · gloss) fed by the unchangedtranslitannotation; the reading line now renders each token as script only (its<bdi>+ ḥarakāt fade + status wash + click).DESIGN.mdwas already corrected in this branch (the new### Reading pedagogy — the binding lawsubsection + the reworked Tokens / word-card / known-gaps entries); this PR is the code + fixtures catching up to that binding law. Committed as the first commit here.Screenshots
Local Playwright render of
/?fixture=ar-blend-intro&delayMs=0against a locally-built site + fixtures server (no deploy). Not committing PNGs (per screenshot policy); the DOM evidence from the drive:Reading line (script only):
Let's ease in. Picture a slow morning — you slip into the سُوق before the stalls fill up, order a small قَهوة from the first vendor who's awake, and sip it on a step while the rest of the مدينة stirs.— nodata-translit, nolang="ar-Latn"sub-label, and nosūq/qahwaormarket/coffee/citygloss text anywhere inline. (قَهوة / مدينة show fewer ḥarakāt than their diacritized form — the expected learning/known fade, not a regression.)Click سُوق → card: panel row order is exactly
root(س و ق) · pattern(فُعْل) · pronunciation(sūq — lang="ar-Latn", JetBrains Mono, muted-soft rgb(180,172,157)) · gloss(market · NOUN).browser_console_messages(error): only a pre-existingfavicon.ico404 — no app errors/warnings.Local Playwright drive at desktop viewport (default); mobile not separately captured (removal-only UI change, no new responsive surface)
Matches the design language in
DESIGN.md— the reading-pedagogy binding law; pronunciation styled with thetransliterationtype tokensNo Figma update needed— canonical frame77:2already shows the card-reveal model; the shipped build was what diverged, so this PR reconverges the build to the existing frame (no visible delta to push back)Test plan
pnpm run build && pnpm run typecheck && pnpm run test— all green (build 8/8; typecheck 8/8; tests: lang-ar 47, web 39, server 54 + 9 skipped, plus lang-fake / lang-zh / conformance — all pass)pnpm run boundary:renderer-neutrality+pnpm run boundary:lang-conditional— PASSpnpm run spec:generate:nodethengit diff --exit-codeonar-annotations.schema.json— clean (schema untouched; thetranslitfield stays, it now feeds the card)ar-word-token.test.tsxflipped to assert no inline romanization for any status + thatArBreakdownPanelshows the pronunciation row when opened (and omits it whentranslitabsent); base-direction tests untouchedrenderer.spec.ts(on the untouchedar-mixed-linefixture); thear-blend-introbehavior was verified via a local MCP smoke drive (above)DESIGN.mdupdated (the binding law, committed here);tokens.jsonuntouched — the pronunciation row reuses existing--la-font-mono/--la-type-annotation-size/--la-color-muted-softtokens (no new/changed token);No Figma update needed(frame77:2already shows the card-reveal model).AGENTS.md/CLAUDE.mdnot touched — shim check N/A.Plan / issue reference
Out of plan — corrective fix for the reading-pedagogy violation shipped in #46, reconverging the build to
DESIGN.md's new binding law (conversation-not-lesson; translation is click/hover-only).🤖 Generated with Claude Code