github-starred/cs249r_book
2
0
Fork 0
mirror of https://github.com/harvard-edge/cs249r_book.git synced 2026-07-16 14:42:29 -05:00
Code Issues 276 Packages Projects Releases 26 Wiki Activity

[PR #1912] [MERGED] fix(staffml): keep practice-page Cmd+Enter handler fresh via latest-callback refs #36527

New Issue
Closed
opened 2026-07-16 00:25:56 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-07-16 00:25:56 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/harvard-edge/cs249r_book/pull/1912
Author: @farhan523
Created: 6/28/2026
Status: ✅ Merged
Merged: 7/10/2026
Merged by: @profvjreddi

Base: dev ← Head: fix/practice-stale-userAnswer

📝 Commits (1)

  • 9ea0c6a fix(staffml): keep practice-page Cmd+Enter handler fresh via latest-callback refs

📊 Changes

1 file changed (+28 additions, -3 deletions)

View changed files

📝 interviews/staffml/src/app/practice/page.tsx (+28 -3)

📄 Description

Summary

The global keyboard listener in practice/page.tsx is intentionally stable — re-armed only when showAnswer/current/pickRandom change so it doesn't churn on every keystroke. But the handlers it calls close over state that DOES change between listener re-arms:

Bug 1 (the one that surfaces visibly): stale userAnswer

  • handleReveal (line 460+) closes over userAnswer.
  • User types 100+ chars in the textarea → setUserAnswer re-renders → fresh handleReveal exists in the new render, but the keydown effect's deps ([showAnswer, current, pickRandom]) didn't change so the effect doesn't re-run.
  • Listener still holds the OLD handleReveal which closes over the OLD userAnswer (empty string).
  • User presses ⌘/Ctrl+Enter in the textarea → stale handler fires → charsTyped = userAnswer.trim().length = 0 → think-guard at line 466 misfires the "are you sure?" modal AFTER the user has actually deliberated.

Bug 2 (same class, smaller blast radius): stale effectiveMaxScore

  • handleScore (line 545+) closes over effectiveMaxScore (derived from rubricItems / napkinResult).
  • User reveals the answer (showAnswer=true re-arms the listener) → ticks rubric checkboxes → effectiveMaxScore changes → digit-key listener still calls the stale handleScore that caps the score at the OLD max.

The Reveal/score buttons at lines 1190/1361/1397/1489 are unaffected — they invoke a fresh closure on click.

Fix

Latest-callback ref pattern (same shape as useVisibilityPoll from #1834):

const handleRevealRef = useRef<(force?: boolean) => void>(() => {});
const handleScoreRef = useRef<(score: number) => void>(() => {});

// Refresh on every render — after handleReveal / handleScore are defined.
useEffect(() => {
  handleRevealRef.current = handleReveal;
  handleScoreRef.current = handleScore;
});

// Listener stays stable, but calls always see the latest closures.
useEffect(() => {
  const handleKeyDown = (e: KeyboardEvent) => {
    // ...
    handleRevealRef.current();
    // ...
    handleScoreRef.current(parseInt(e.key) - 1);
  };
  // ...
}, [showAnswer, current, pickRandom]);

Inline comment in both spots documents the trap so a future "cleanup" doesn't undo it.

Test plan

  • npx vitest run → 126/126 across 22 files passing (unchanged from dev; no new tests added — see Notes).
  • npx tsc --noEmit → clean.
  • Manual repro (before/after):
    1. Open /practice. Wait for a question to load.
    2. Type a 100-character answer into the textarea (genuinely think through it for >20s).
    3. Press ⌘/Ctrl+Enter.
    4. Before: think-guard modal pops up ("Are you sure you want to reveal?") despite the deliberate input.
    5. After: reveal proceeds directly.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/harvard-edge/cs249r_book/pull/1912 **Author:** [@farhan523](https://github.com/farhan523) **Created:** 6/28/2026 **Status:** ✅ Merged **Merged:** 7/10/2026 **Merged by:** [@profvjreddi](https://github.com/profvjreddi) **Base:** `dev` ← **Head:** `fix/practice-stale-userAnswer` --- ### 📝 Commits (1) - [`9ea0c6a`](https://github.com/harvard-edge/cs249r_book/commit/9ea0c6a7e0d8ab436a75497e6309c246265e580c) fix(staffml): keep practice-page Cmd+Enter handler fresh via latest-callback refs ### 📊 Changes **1 file changed** (+28 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `interviews/staffml/src/app/practice/page.tsx` (+28 -3) </details> ### 📄 Description ## Summary The global keyboard listener in `practice/page.tsx` is intentionally stable — re-armed only when `showAnswer`/`current`/`pickRandom` change so it doesn't churn on every keystroke. But the handlers it calls close over state that DOES change between listener re-arms: ### Bug 1 (the one that surfaces visibly): stale `userAnswer` - `handleReveal` ([line 460+](src/app/practice/page.tsx#L460)) closes over `userAnswer`. - User types 100+ chars in the textarea → `setUserAnswer` re-renders → fresh `handleReveal` exists in the new render, but the keydown effect's deps (`[showAnswer, current, pickRandom]`) didn't change so the effect doesn't re-run. - Listener still holds the OLD `handleReveal` which closes over the OLD `userAnswer` (empty string). - User presses **⌘/Ctrl+Enter** in the textarea → stale handler fires → `charsTyped = userAnswer.trim().length = 0` → think-guard at [line 466](src/app/practice/page.tsx#L466) misfires the \"are you sure?\" modal AFTER the user has actually deliberated. ### Bug 2 (same class, smaller blast radius): stale `effectiveMaxScore` - `handleScore` ([line 545+](src/app/practice/page.tsx#L545)) closes over `effectiveMaxScore` (derived from `rubricItems` / `napkinResult`). - User reveals the answer (`showAnswer=true` re-arms the listener) → ticks rubric checkboxes → `effectiveMaxScore` changes → digit-key listener still calls the stale `handleScore` that caps the score at the OLD max. The Reveal/score **buttons** at lines 1190/1361/1397/1489 are unaffected — they invoke a fresh closure on click. ### Fix Latest-callback ref pattern (same shape as `useVisibilityPoll` from #1834): ```ts const handleRevealRef = useRef<(force?: boolean) => void>(() => {}); const handleScoreRef = useRef<(score: number) => void>(() => {}); // Refresh on every render — after handleReveal / handleScore are defined. useEffect(() => { handleRevealRef.current = handleReveal; handleScoreRef.current = handleScore; }); // Listener stays stable, but calls always see the latest closures. useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { // ... handleRevealRef.current(); // ... handleScoreRef.current(parseInt(e.key) - 1); }; // ... }, [showAnswer, current, pickRandom]); ``` Inline comment in both spots documents the trap so a future \"cleanup\" doesn't undo it. ## Test plan - [x] `npx vitest run` → 126/126 across 22 files passing (unchanged from `dev`; no new tests added — see Notes). - [x] `npx tsc --noEmit` → clean. - [ ] Manual repro (before/after): 1. Open `/practice`. Wait for a question to load. 2. Type a 100-character answer into the textarea (genuinely think through it for >20s). 3. Press **⌘/Ctrl+Enter**. 4. **Before**: think-guard modal pops up (\"Are you sure you want to reveal?\") despite the deliberate input. 5. **After**: reveal proceeds directly. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-07-16 00:25:56 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area: book
area: collabs
area: kits
area: labs
area: socratiq
area: staffml
area: tinytorch
area: tools
area: website
bug
dependencies
format: epub
format: pdf
javascript
link-health
link-rot
priority-high
pull-request
Mirrored from GitHub Pull Request
 staffml
type: bug
type: citation
type: code
type: errata
type: improvement
type: new
type: question
vault-sli
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/cs249r_book#36527
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?