Code-review fixes for collocation coach + vocab garden

Correctness:
- useCheckpoint: clear the busy flag unconditionally so overlapping
  explicit passes don't strand each other's spinner; explicit actions
  now also supersede a queued auto-check and clear the stranded
  "checking" dot. Deduped runVoice/runCollocation into runExplicitPass.
- EditorCore: token-guard the auto-capture so a late capture can't
  resurrect a removed word; move toggleSaveWord side effects out of the
  setWordInfo updater (StrictMode double-fire); fix sentenceAround offset
  desync via shared exampleAt (textBetween + parentOffset, single resolve);
  optimistic saved state so the heart doesn't flash unsaved.
- vocab capture: normalize word to lower+trim (matches lexicon) so
  "Apple"/"apple" don't make duplicate cards; check rows.Err() in queryList.
- GardenPanel: Promise.allSettled so a /due failure doesn't blank the
  whole garden; scrim click during review ends the review (mirrors Esc);
  gate footer on !error; O(1) due lookup via a Set.

Features requested in review:
- Definition-only review card: add vocab_words.definition (migration
  0007) as an English fallback meaning, threaded through capture and used
  by review/garden when there's no Chinese gloss.
- Scheduler caps: maxEase 3.0 + maxInterval 365d so "easy" growth can't
  push a word out of rotation for years.

Tests: TestCaptureCaseInsensitive, TestCaptureStoresDefinitionFallback,
TestCapsBoundGrowth. go build/vet/test, tsc, vitest 51/51, vite build clean.

Claude-Session: https://claude.ai/code/session_016Yr6jELuRc7hyzYLccQKZd
This commit is contained in:
prosolis
2026-06-26 16:41:28 -07:00
parent 8aa437ec82
commit 4161830da6
9 changed files with 256 additions and 98 deletions

View File

@@ -325,6 +325,17 @@ CREATE TABLE vocab_words (
);
CREATE INDEX idx_vocab_due ON vocab_words(user_id, due_at);
`,
},
{
// Vocabulary garden, follow-up. Some looked-up words have an English
// definition but no Chinese gloss; those produced an unanswerable review
// card (review reveals only the gloss). `definition` stores a short
// English sense captured at lookup as a fallback "meaning" so such words
// are still reviewable.
name: "0007_vocab_definition",
stmt: `
ALTER TABLE vocab_words ADD COLUMN definition TEXT NOT NULL DEFAULT '';
`,
},
}

View File

@@ -20,6 +20,7 @@ type Word struct {
ID string `json:"id"`
Word string `json:"word"`
Gloss string `json:"gloss"`
Definition string `json:"definition"` // English fallback meaning when there's no Chinese gloss
Phonetic string `json:"phonetic"`
Example string `json:"example"`
DocID *string `json:"doc_id"`
@@ -51,7 +52,7 @@ func (h *Handler) Routes() chi.Router {
return r
}
const vocabColumns = `id, word, gloss, phonetic, example, doc_id,
const vocabColumns = `id, word, gloss, definition, phonetic, example, doc_id,
due_at, interval_days, ease, reps, lapses, last_reviewed, created_at`
func scanWord(s interface {
@@ -59,7 +60,7 @@ func scanWord(s interface {
}) (Word, error) {
var w Word
err := s.Scan(
&w.ID, &w.Word, &w.Gloss, &w.Phonetic, &w.Example, &w.DocID,
&w.ID, &w.Word, &w.Gloss, &w.Definition, &w.Phonetic, &w.Example, &w.DocID,
&w.DueAt, &w.IntervalDays, &w.Ease, &w.Reps, &w.Lapses, &w.LastReviewed, &w.CreatedAt,
)
return w, err
@@ -93,15 +94,20 @@ func (h *Handler) queryList(w http.ResponseWriter, query string, args ...any) {
}
out = append(out, word)
}
if err := rows.Err(); err != nil {
serverError(w, err)
return
}
writeJSON(w, http.StatusOK, out)
}
type captureRequest struct {
Word string `json:"word"`
Gloss string `json:"gloss"`
Phonetic string `json:"phonetic"`
Example string `json:"example"`
DocID *string `json:"doc_id"`
Word string `json:"word"`
Gloss string `json:"gloss"`
Definition string `json:"definition"`
Phonetic string `json:"phonetic"`
Example string `json:"example"`
DocID *string `json:"doc_id"`
}
// capture records a looked-up word. It's an idempotent upsert keyed on the word:
@@ -114,7 +120,11 @@ func (h *Handler) capture(w http.ResponseWriter, r *http.Request) {
errorJSON(w, http.StatusBadRequest, "invalid body")
return
}
word := strings.TrimSpace(req.Word)
// Normalize the same way the lexicon does (internal/lexicon: lower+trim) so
// the UNIQUE(user_id, word) upsert is genuinely idempotent — otherwise
// "Apple" at a sentence start and "apple" mid-line would create two separate
// cards on independent schedules.
word := strings.ToLower(strings.TrimSpace(req.Word))
if word == "" {
errorJSON(w, http.StatusBadRequest, "word is required")
return
@@ -124,14 +134,15 @@ func (h *Handler) capture(w http.ResponseWriter, r *http.Request) {
// schedule (due_at/reps/interval/ease) alone so re-looking-up a word never
// resets its progress.
_, err := h.DB.Exec(
`INSERT INTO vocab_words (user_id, word, gloss, phonetic, example, doc_id, due_at, interval_days)
VALUES (?, ?, ?, ?, ?, ?, datetime('now', '+1 day'), 1)
`INSERT INTO vocab_words (user_id, word, gloss, definition, phonetic, example, doc_id, due_at, interval_days)
VALUES (?, ?, ?, ?, ?, ?, ?, datetime('now', '+1 day'), 1)
ON CONFLICT(user_id, word) DO UPDATE SET
gloss = excluded.gloss,
phonetic = excluded.phonetic,
example = CASE WHEN excluded.example != '' THEN excluded.example ELSE vocab_words.example END,
doc_id = COALESCE(excluded.doc_id, vocab_words.doc_id)`,
db.LocalUserID, word, req.Gloss, req.Phonetic, req.Example, req.DocID,
gloss = excluded.gloss,
definition = excluded.definition,
phonetic = excluded.phonetic,
example = CASE WHEN excluded.example != '' THEN excluded.example ELSE vocab_words.example END,
doc_id = COALESCE(excluded.doc_id, vocab_words.doc_id)`,
db.LocalUserID, word, req.Gloss, req.Definition, req.Phonetic, req.Example, req.DocID,
)
if err != nil {
serverError(w, err)

View File

@@ -141,6 +141,48 @@ func TestCaptureValidation(t *testing.T) {
}
}
// TestCaptureStoresDefinitionFallback proves a word with an English definition
// but no Chinese gloss still round-trips its definition, so review has a meaning
// to reveal instead of a blank card.
func TestCaptureStoresDefinitionFallback(t *testing.T) {
srv, _ := newTestServer(t)
rec := do(t, srv, http.MethodPost, "/vocab",
`{"word":"ineffable","definition":"too great to be expressed in words"}`)
if rec.Code != http.StatusCreated {
t.Fatalf("capture: code=%d body=%s", rec.Code, rec.Body)
}
var w Word
_ = json.Unmarshal(rec.Body.Bytes(), &w)
if w.Gloss != "" {
t.Fatalf("expected no gloss, got %q", w.Gloss)
}
if w.Definition != "too great to be expressed in words" {
t.Fatalf("definition not stored, got %q", w.Definition)
}
}
// TestCaptureCaseInsensitive proves "Apple" (sentence start) and "apple"
// (mid-line) land in the SAME garden card — capture normalizes to lower+trim
// like the lexicon, so the idempotent upsert isn't defeated by casing.
func TestCaptureCaseInsensitive(t *testing.T) {
srv, _ := newTestServer(t)
if rec := do(t, srv, http.MethodPost, "/vocab", `{"word":"Apple","gloss":"苹果"}`); rec.Code != http.StatusCreated {
t.Fatalf("capture Apple: code=%d body=%s", rec.Code, rec.Body)
}
if rec := do(t, srv, http.MethodPost, "/vocab", `{"word":" apple ","gloss":"苹果"}`); rec.Code != http.StatusCreated {
t.Fatalf("capture apple: code=%d body=%s", rec.Code, rec.Body)
}
rec := do(t, srv, http.MethodGet, "/vocab", "")
var all []Word
_ = json.Unmarshal(rec.Body.Bytes(), &all)
if len(all) != 1 {
t.Fatalf("Apple/apple should be one card, got %d", len(all))
}
if all[0].Word != "apple" {
t.Fatalf("word should be normalized to lowercase, got %q", all[0].Word)
}
}
// TestDocLinkSurvivesDocDelete proves the ON DELETE SET NULL keeps a word in the
// garden when its source document is removed.
func TestDocLinkSurvivesDocDelete(t *testing.T) {

View File

@@ -23,10 +23,17 @@ var ladder = []int{1, 3, 7, 16, 35}
const (
minEase = 1.3
maxEase = 3.0 // ease ceiling — keeps "easy" from compounding growth without bound
startEase = 2.5
easyBonus = 1.3 // multiplier applied on top of a "good" step for "easy"
easeAgainDelta = -0.20 // ease nudged down on a lapse (still floored at minEase)
easeEasyDelta = 0.15 // ease nudged up when a card feels easy
// maxInterval caps how far a card can drift into the future. Even a word
// graded "easy" many times resurfaces about once a year, so nothing silently
// leaves the garden forever (the whole point is to keep words in gentle
// rotation, not to retire them).
maxInterval = 365
)
// State is the spaced-repetition state of one card. It mirrors the scheduling
@@ -61,7 +68,7 @@ func (s State) next(grade Grade) State {
reps := s.Reps + 1
return State{
Reps: reps,
Interval: int(math.Round(float64(goodInterval(reps, s.Interval, ease)) * easyBonus)),
Interval: clampInterval(int(math.Round(float64(goodInterval(reps, s.Interval, ease)) * easyBonus))),
Ease: ease,
Lapses: s.Lapses,
}
@@ -69,7 +76,7 @@ func (s State) next(grade Grade) State {
reps := s.Reps + 1
return State{
Reps: reps,
Interval: goodInterval(reps, s.Interval, ease),
Interval: clampInterval(goodInterval(reps, s.Interval, ease)),
Ease: ease,
Lapses: s.Lapses,
}
@@ -93,5 +100,15 @@ func clampEase(e float64) float64 {
if e < minEase {
return minEase
}
if e > maxEase {
return maxEase
}
return e
}
func clampInterval(d int) int {
if d > maxInterval {
return maxInterval
}
return d
}

View File

@@ -51,6 +51,28 @@ func TestAgainStepsBackGently(t *testing.T) {
}
}
// TestCapsBoundGrowth proves runaway "easy" grading can't push ease or the
// interval past their ceilings, so a word always resurfaces within a year.
func TestCapsBoundGrowth(t *testing.T) {
s := State{Ease: startEase}
for i := 0; i < 40; i++ {
s = s.next(GradeEasy)
if s.Ease > maxEase {
t.Fatalf("step %d: ease %v exceeded cap %v", i, s.Ease, maxEase)
}
if s.Interval > maxInterval {
t.Fatalf("step %d: interval %d exceeded cap %d", i, s.Interval, maxInterval)
}
}
// After enough "easy" reviews it should be pinned at the ceilings.
if s.Interval != maxInterval {
t.Fatalf("interval should saturate at %d, got %d", maxInterval, s.Interval)
}
if s.Ease != maxEase {
t.Fatalf("ease should saturate at %v, got %v", maxEase, s.Ease)
}
}
// TestEasyAdvancesFurther proves "easy" both bumps ease and lands a longer
// interval than a plain "good" at the same rung.
func TestEasyAdvancesFurther(t *testing.T) {

View File

@@ -85,6 +85,7 @@ export interface VocabWord {
id: string
word: string
gloss: string
definition: string // English fallback meaning shown in review when there's no gloss
phonetic: string
example: string
doc_id: string | null
@@ -254,6 +255,7 @@ export const api = {
recordVocab: (body: {
word: string
gloss?: string
definition?: string
phonetic?: string
example?: string
doc_id?: string | null

View File

@@ -640,6 +640,20 @@ export function EditorCore({
setMisspell(null)
}, [misspell, onAddWord])
// exampleAt pulls the sentence containing the position out of its block, for
// review context in the garden. textBetween with a single-char leaf/break
// placeholder keeps the string indices aligned with ProseMirror's parentOffset
// (so a hard break or inline atom before the word doesn't desync the slice).
const exampleAt = useCallback(
(pos: number): string => {
if (!editor) return ''
const $pos = editor.state.doc.resolve(pos)
const text = $pos.parent.textBetween(0, $pos.parent.content.size, '\n', ' ')
return sentenceAround(text, Math.max(0, $pos.parentOffset))
},
[editor],
)
// openWordLookup resolves the exact word span at a document position, anchors a
// popover beneath it, and kicks off the offline lookup. The card opens
// immediately in a loading state and fills in when the (local) lookup returns.
@@ -665,28 +679,35 @@ export function EditorCore({
const token = ++wordReqRef.current
setWordInfo({ word: range.word, from: range.from, to: range.to, top, left, loading: true, info: null, vocabId: null, saved: false })
// The sentence the word sits in, for review context in the garden.
const block = editor.state.doc.resolve(range.from).parent.textContent
const example = sentenceAround(block, Math.max(0, range.from - editor.state.doc.resolve(range.from).start()))
const example = exampleAt(range.from)
api
.lookupWord(range.word)
.then((info) => {
if (token !== wordReqRef.current) return
setWordInfo((w) => (w ? { ...w, loading: false, info } : null))
// Auto-capture into the vocabulary garden — only words the dictionary
// actually knows (a real gloss or definition), so accidental lookups of
// typos or proper nouns don't clutter the garden. Looking words up IS
// the data source; this costs the writer nothing.
const known = !!info.gloss || info.definitions.length > 0
// Reflect the saved state optimistically so the heart shows 💚 the
// moment a known word loads, rather than flashing 🤍 until the capture
// round-trips. vocabId is filled in when recordVocab returns.
setWordInfo((w) => (w ? { ...w, loading: false, info, saved: known } : null))
if (!known) return
api
.recordVocab({
word: range.word,
gloss: info.gloss,
definition: info.definitions[0]?.definition ?? '',
phonetic: info.phonetic,
example,
doc_id: docId,
})
.then((row) => {
// Discard a late capture if the card has since been superseded (a
// new lookup, navigation, or an explicit remove all bump the token),
// so it can't resurrect a word the writer just removed.
if (token !== wordReqRef.current) return
setWordInfo((w) => (w && w.word === range.word ? { ...w, vocabId: row.id, saved: true } : w))
})
.catch((err) => console.error('vocab capture failed', err))
@@ -705,23 +726,36 @@ export function EditorCore({
// heart. Auto-capture saves it on lookup; this lets the writer remove a word
// she already knows (or re-add one she removed by mistake).
const toggleSaveWord = useCallback(() => {
setWordInfo((w) => {
if (!w || !w.info) return w
if (w.saved && w.vocabId) {
const id = w.vocabId
api.deleteVocab(id).catch((err) => console.error('vocab remove failed', err))
return { ...w, saved: false, vocabId: null }
}
const word = w.word
const block = editor?.state.doc.resolve(w.from)
const example = block ? sentenceAround(block.parent.textContent, Math.max(0, w.from - block.start())) : ''
api
.recordVocab({ word, gloss: w.info.gloss, phonetic: w.info.phonetic, example, doc_id: docId })
.then((row) => setWordInfo((cur) => (cur && cur.word === word ? { ...cur, vocabId: row.id, saved: true } : cur)))
.catch((err) => console.error('vocab save failed', err))
return { ...w, saved: true }
})
}, [editor, docId])
// Read the current card and do the network side effects OUTSIDE the state
// updater — an updater must be pure (React StrictMode double-invokes it,
// which would otherwise fire each request twice).
const w = wordInfo
if (!w || !w.info) return
if (w.vocabId) {
// Already in the garden — remove it, and invalidate any in-flight capture
// for this card so a late auto-capture can't resurrect the removed word.
const id = w.vocabId
wordReqRef.current++
setWordInfo((cur) => (cur ? { ...cur, saved: false, vocabId: null } : cur))
api.deleteVocab(id).catch((err) => console.error('vocab remove failed', err))
return
}
// Not in the garden yet — save it (idempotent upsert keyed on the word).
const word = w.word
const example = exampleAt(w.from)
setWordInfo((cur) => (cur ? { ...cur, saved: true } : cur))
api
.recordVocab({
word,
gloss: w.info.gloss,
definition: w.info.definitions[0]?.definition ?? '',
phonetic: w.info.phonetic,
example,
doc_id: docId,
})
.then((row) => setWordInfo((cur) => (cur && cur.word === word ? { ...cur, vocabId: row.id, saved: true } : cur)))
.catch((err) => console.error('vocab save failed', err))
}, [wordInfo, exampleAt, docId])
// Right-click a word to look it up. Right-clicking off any word falls through
// to the native menu (so copy/paste-by-menu still works — see the Selection fix).

View File

@@ -51,13 +51,14 @@ export function GardenPanel({ onClose, onOpenDoc }: Props) {
const load = useCallback(async () => {
setError(false)
try {
const [all, dueNow] = await Promise.all([api.listVocab(), api.dueVocab()])
setWords(all)
setDue(dueNow)
} catch {
setError(true)
}
// Settle the two requests independently: a failed /due shouldn't blank the
// whole garden when the word list loaded fine. Only listVocab failing is a
// true error state; a dueVocab failure just hides the review button.
const [allRes, dueRes] = await Promise.allSettled([api.listVocab(), api.dueVocab()])
if (allRes.status === 'fulfilled') setWords(allRes.value)
else setError(true)
if (dueRes.status === 'fulfilled') setDue(dueRes.value)
else setDue([])
}, [])
useEffect(() => {
@@ -119,7 +120,11 @@ export function GardenPanel({ onClose, onOpenDoc }: Props) {
return (
<div className="petal-no-print fixed inset-0 z-40 flex justify-end">
<div className="absolute inset-0" style={{ background: 'rgba(61, 46, 57, 0.18)' }} onClick={onClose} />
<div
className="absolute inset-0"
style={{ background: 'rgba(61, 46, 57, 0.18)' }}
onClick={() => (queue ? setQueue(null) : onClose())}
/>
<aside
className="relative flex h-full w-full max-w-[420px] flex-col"
@@ -202,6 +207,9 @@ function GardenView({
onOpenDoc?: (docId: string) => void
onRetry: () => void
}) {
// Index the due cards once so the per-word "due" check below is O(1), not a
// linear scan of `due` for every word in the garden.
const dueIds = useMemo(() => new Set(due.map((d) => d.id)), [due])
return (
<>
{due.length > 0 && (
@@ -246,7 +254,7 @@ function GardenView({
<ul className="flex flex-col gap-1.5">
{words.map((w) => {
const open = expanded === w.id
const isDue = due.some((d) => d.id === w.id)
const isDue = dueIds.has(w.id)
return (
<li key={w.id}>
<button
@@ -263,15 +271,17 @@ function GardenView({
</span>
<span className="flex min-w-0 flex-1 flex-col">
<span className="truncate text-sm font-bold text-plum">{w.word}</span>
{w.gloss && (
{(w.gloss || w.definition) && (
<span
className="truncate text-xs"
style={{
color: 'var(--color-muted)',
fontFamily: "'Nunito','PingFang SC','Microsoft YaHei','Noto Sans CJK SC',sans-serif",
fontFamily: w.gloss
? "'Nunito','PingFang SC','Microsoft YaHei','Noto Sans CJK SC',sans-serif"
: 'var(--font-body)',
}}
>
{w.gloss}
{w.gloss || w.definition}
</span>
)}
</span>
@@ -335,7 +345,7 @@ function GardenView({
)}
</div>
{words && words.length > 0 && (
{!error && words && words.length > 0 && (
<div
className="shrink-0 px-4 py-2.5 text-center text-[11px]"
style={{ borderTop: '1px solid var(--color-border)', color: 'var(--color-muted)' }}
@@ -365,16 +375,19 @@ function ReviewSession({
onQuit: () => void
}) {
const card = queue[cursor]
// The meaning shown/asked is the Chinese gloss, or the English definition when
// a word has no gloss — so definition-only words are still reviewable.
const meaning = card?.gloss || card?.definition || ''
// Alternate the quiz direction so she practices both recognition (see the
// English, recall the meaning) and production (see the meaning, recall the
// English word). Parity of the cursor keeps it deterministic within a session.
const production = cursor % 2 === 1 && !!card?.gloss
const production = cursor % 2 === 1 && !!meaning
const prompt = useMemo(() => {
if (!card) return ''
if (production) return card.gloss
if (production) return meaning
return card.example ? blankOut(card.example, card.word) : card.word
}, [card, production])
}, [card, production, meaning])
if (!card) return null
@@ -432,15 +445,19 @@ function ReviewSession({
/{card.phonetic}/
</div>
)}
{card.gloss && (
{meaning && (
<div
className="mt-1 text-sm font-semibold"
style={{
color: 'var(--color-accent-hover)',
fontFamily: "'Nunito','PingFang SC','Microsoft YaHei','Noto Sans CJK SC',sans-serif",
// Chinese gloss gets the CJK stack; an English definition fallback
// reads better in the body font.
fontFamily: card.gloss
? "'Nunito','PingFang SC','Microsoft YaHei','Noto Sans CJK SC',sans-serif"
: 'var(--font-body)',
}}
>
{card.gloss}
{meaning}
</div>
)}
{card.example && (

View File

@@ -68,51 +68,53 @@ export function useCheckpoint(docId: string | null) {
}
}, [])
// Run the voice-consistency pass now (explicit "Check my voice" action). Like
// runCheck it returns the unified pending set, so grammar highlights survive.
// Shares the run token so navigating away discards a late voice response.
const runVoice = useCallback(async () => {
const id = docIdRef.current
if (!id) return
clearTimeout(retryRef.current) // a voice pass supersedes a queued grammar retry
const run = ++runRef.current
setVoicing(true)
try {
const full = await api.voiceDoc(id)
if (run === runRef.current && id === docIdRef.current) {
setSuggestions(full)
setLlmDown(false)
// runExplicitPass drives a whole-document, explicit-action pass (voice,
// collocation): it returns the unified pending set, so the other families'
// highlights survive, and shares the run token so navigating away discards a
// late response. It supersedes any queued grammar retry AND clears the
// checkpoint's "checking" state, so the breathing rose dot can't linger on
// after the retry timer it would have cleared is cancelled here.
const runExplicitPass = useCallback(
async (call: (id: string) => Promise<Suggestion[]>, setBusy: (b: boolean) => void, label: string) => {
const id = docIdRef.current
if (!id) return
// An explicit action fully supersedes a queued auto-check and grammar
// retry, and takes over the indicator — clear the stranded "checking" dot.
clearTimeout(debounceRef.current)
clearTimeout(retryRef.current)
setChecking(false)
const run = ++runRef.current
setBusy(true)
try {
const full = await call(id)
if (run === runRef.current && id === docIdRef.current) {
setSuggestions(full)
setLlmDown(false)
}
} catch (err) {
console.error(`${label} failed`, err)
if (run === runRef.current) setLlmDown(true)
} finally {
// setBusy is THIS invocation's own flag (a newer run sets its own), so
// clear it unconditionally — otherwise an overlapping pass that bumped
// the run token would strand this spinner on "Reading…" forever.
setBusy(false)
}
} catch (err) {
console.error('voice pass failed', err)
if (run === runRef.current) setLlmDown(true)
} finally {
if (run === runRef.current) setVoicing(false)
}
}, [])
},
[],
)
// Run the voice-consistency pass now (explicit "Check my voice" action).
const runVoice = useCallback(
() => runExplicitPass(api.voiceDoc, setVoicing, 'voice pass'),
[runExplicitPass],
)
// Run the collocation coach now (explicit "Make it sound natural" action).
// Like runVoice it returns the unified pending set, so grammar + voice
// highlights survive. Shares the run token so a late response is discarded.
const runCollocation = useCallback(async () => {
const id = docIdRef.current
if (!id) return
clearTimeout(retryRef.current) // a collocation pass supersedes a queued grammar retry
const run = ++runRef.current
setCollocating(true)
try {
const full = await api.collocationDoc(id)
if (run === runRef.current && id === docIdRef.current) {
setSuggestions(full)
setLlmDown(false)
}
} catch (err) {
console.error('collocation pass failed', err)
if (run === runRef.current) setLlmDown(true)
} finally {
if (run === runRef.current) setCollocating(false)
}
}, [])
const runCollocation = useCallback(
() => runExplicitPass(api.collocationDoc, setCollocating, 'collocation pass'),
[runExplicitPass],
)
// Call on every edit; schedules a check 4s after typing settles.
const schedule = useCallback(() => {