Skip to content

Commit 0acfac6

Browse files
jeremymanningclaude
andcommitted
Fix critical GP numerical stability, import/export, and quiz UX bugs
- Fix NaN/red-collapse: increase Cholesky jitter from 1e-10 to 1e-6, add NaN fallback in choleskySolve (returns zero vector instead of NaN), add isFinite guards in estimator predict, $coverage, updateConfidence, and computeConceptKnowledge - Fix JSON import: keep FileReader input reference to prevent GC, add onerror handler, include x/y coordinates in exports, look up missing coords from questionIndex during import - Replace auto-advance with explicit "Next" button after answering, giving users time to read correct answer feedback - Add "Learn more" buttons (Wikipedia + Khan Academy) after each answer - Add keyboard shortcut (Enter/N/Space) to advance to next question - Fix knowledge bars in expertise/learning modals (NaN propagation) - Fix race condition causing wrong answer options for different question (eliminated by removing setTimeout chain in answer flow) Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com>
1 parent 972c981 commit 0acfac6

File tree

9 files changed

+191
-27
lines changed

9 files changed

+191
-27
lines changed

src/app.js

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ async function boot() {
115115
const quizPanel = document.getElementById('quiz-panel');
116116
quiz.init(quizPanel);
117117
quiz.onAnswer(handleAnswer);
118+
quiz.onNext(() => selectAndShowNextQuestion());
118119

119120
renderer.onReanswer((questionId) => {
120121
const q = questionIndex.get(questionId);
@@ -450,10 +451,7 @@ function handleAnswer(selectedKey, question) {
450451

451452
const coverage = Math.round($coverage.get() * 100);
452453
announce(`${feedback} ${coverage}% of domain mapped. ${50 - domainQuestionCount} questions remaining.`);
453-
454-
setTimeout(() => {
455-
selectAndShowNextQuestion();
456-
}, 900);
454+
// No auto-advance — user clicks "Next" button in quiz panel
457455
}
458456

459457
function handleReset() {
@@ -523,9 +521,26 @@ function handleImport(data) {
523521
return;
524522
}
525523

524+
// Enrich imported responses with x/y coordinates from questionIndex
525+
// (older exports may lack these, but we need them for the GP estimator)
526+
let coordsRecovered = 0;
527+
const enrichedValid = valid.map(r => {
528+
if (r.x != null && r.y != null) return r;
529+
const q = questionIndex.get(r.question_id);
530+
if (q && q.x != null && q.y != null) {
531+
coordsRecovered++;
532+
return { ...r, x: q.x, y: q.y };
533+
}
534+
return r;
535+
});
536+
537+
if (coordsRecovered > 0) {
538+
console.log(`[import] Recovered x/y coordinates for ${coordsRecovered} responses from question index`);
539+
}
540+
526541
const existing = $responses.get();
527542
const existingIds = new Set(existing.map(r => r.question_id));
528-
const newResponses = valid.filter(r => !existingIds.has(r.question_id));
543+
const newResponses = enrichedValid.filter(r => !existingIds.has(r.question_id));
529544
const merged = [...existing, ...newResponses];
530545

531546
$responses.set(merged);

src/learning/estimator.js

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,11 @@ export class Estimator {
145145
state = 'estimated';
146146
}
147147

148-
results[i] = { gx: cell.gx, gy: cell.gy, value, uncertainty, evidenceCount, state };
148+
// NaN safety: if numerical instability produced bad values, use prior
149+
const safeValue = isFinite(value) ? value : PRIOR_MEAN;
150+
const safeUncertainty = isFinite(uncertainty) ? uncertainty : 1.0;
151+
152+
results[i] = { gx: cell.gx, gy: cell.gy, value: safeValue, uncertainty: safeUncertainty, evidenceCount, state };
149153
}
150154

151155
return results;
@@ -201,7 +205,11 @@ export class Estimator {
201205
state = 'estimated';
202206
}
203207

204-
return { gx, gy, value, uncertainty, evidenceCount, state };
208+
// NaN safety: if numerical instability produced bad values, use prior
209+
const safeValue = isFinite(value) ? value : PRIOR_MEAN;
210+
const safeUncertainty = isFinite(uncertainty) ? uncertainty : 1.0;
211+
212+
return { gx, gy, value: safeValue, uncertainty: safeUncertainty, evidenceCount, state };
205213
}
206214

207215
/**
@@ -291,6 +299,11 @@ export class Estimator {
291299
}
292300

293301
this._alpha = choleskySolve(this._K_noisy, this._obsValues);
302+
303+
// Safety: if Cholesky returned a zero vector (NaN fallback), log a warning
304+
if (this._alpha.every(v => v === 0) && this._obsValues.some(v => v !== 0)) {
305+
console.warn('[estimator] Cholesky returned fallback zero vector — predictions will use prior mean');
306+
}
294307
}
295308

296309
/**

src/state/persistence.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export function exportResponses() {
2929
selected: r.selected,
3030
is_correct: r.is_correct,
3131
timestamp: r.timestamp,
32+
x: r.x,
33+
y: r.y,
3234
})),
3335
};
3436
return new Blob([JSON.stringify(data, null, 2)], { type: 'application/json' });

src/state/store.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,12 @@ export const $coverage = computed($estimates, (estimates) => {
4747
let totalCoverage = 0;
4848
for (const e of estimates) {
4949
if (e.state === 'unknown') continue;
50-
// Each cell contributes (1 - uncertainty) — well-evidenced cells contribute more
51-
totalCoverage += (1 - e.uncertainty);
50+
const contrib = 1 - e.uncertainty;
51+
// Guard against NaN from numerical instability in GP
52+
if (isFinite(contrib)) totalCoverage += contrib;
5253
}
53-
return totalCoverage / estimates.length;
54+
const result = totalCoverage / estimates.length;
55+
return isFinite(result) ? result : 0;
5456
});
5557

5658
/** Whether insights are available (>= 10 responses) */

src/ui/controls.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,14 @@ export function init(headerElement) {
206206
importButton.title = 'Import saved progress';
207207
importButton.innerHTML = '<i class="fa-solid fa-upload"></i>';
208208
importButton.hidden = true;
209+
// Keep a module-level reference to avoid GC before FileReader fires
210+
let _importInput = null;
211+
209212
importButton.addEventListener('click', () => {
210-
const input = document.createElement('input');
211-
input.type = 'file';
212-
input.accept = '.json,application/json';
213-
input.addEventListener('change', (e) => {
213+
_importInput = document.createElement('input');
214+
_importInput.type = 'file';
215+
_importInput.accept = '.json,application/json';
216+
_importInput.addEventListener('change', (e) => {
214217
const file = e.target.files[0];
215218
if (!file) return;
216219
const reader = new FileReader();
@@ -222,10 +225,16 @@ export function init(headerElement) {
222225
console.error('[controls] Failed to parse import file:', err);
223226
alert('Invalid file format. Please select a Knowledge Mapper export JSON file.');
224227
}
228+
_importInput = null; // Release reference after successful read
229+
};
230+
reader.onerror = () => {
231+
console.error('[controls] FileReader error:', reader.error);
232+
alert('Could not read file. Please try again.');
233+
_importInput = null;
225234
};
226235
reader.readAsText(file);
227236
});
228-
input.click();
237+
_importInput.click();
229238
});
230239
container.appendChild(importButton);
231240

src/ui/insights.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,8 @@ export function computeConceptKnowledge(estimates, region, gridSize, opts = {})
222222
function knowledgeAt(x, y) {
223223
const gx = Math.min(gridSize - 1, Math.max(0, Math.floor((x - region.x_min) / cellW)));
224224
const gy = Math.min(gridSize - 1, Math.max(0, Math.floor((y - region.y_min) / cellH)));
225-
return estimateMap.get(`${gx},${gy}`) ?? 0.5;
225+
const val = estimateMap.get(`${gx},${gy}`) ?? 0.5;
226+
return isFinite(val) ? val : 0.5; // Guard against NaN from GP instability
226227
}
227228

228229
const results = [];
@@ -231,7 +232,8 @@ export function computeConceptKnowledge(estimates, region, gridSize, opts = {})
231232
for (const c of pts) {
232233
sum += knowledgeAt(c.x, c.y);
233234
}
234-
results.push({ concept, knowledge: sum / pts.length, count: pts.length });
235+
const knowledge = pts.length > 0 ? sum / pts.length : 0.5;
236+
results.push({ concept, knowledge: isFinite(knowledge) ? knowledge : 0.5, count: pts.length });
235237
}
236238

237239
return results;

src/ui/progress.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,10 @@ export function initConfidence(container) {
113113

114114
export function updateConfidence(coverage) {
115115
if (!confidenceFill || !confidenceText) return;
116-
117-
const pct = Math.min(100, Math.max(0, coverage * 100));
116+
117+
// Guard against NaN from GP numerical instability
118+
const safeCoverage = isFinite(coverage) ? coverage : 0;
119+
const pct = Math.min(100, Math.max(0, safeCoverage * 100));
118120
confidenceFill.style.width = `${pct}%`;
119121
confidenceText.textContent = `Domain mapped: ${Math.round(pct)}%`;
120122
}

src/ui/quiz.js

Lines changed: 115 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { announce } from '../utils/accessibility.js';
44

55
let answerCallback = null;
6+
let nextCallback = null;
67
let currentQuestion = null;
78
let uiElements = {};
89

@@ -110,6 +111,50 @@ export function init(container) {
110111
.quiz-meta a:hover {
111112
text-shadow: 0 0 6px var(--color-glow-secondary);
112113
}
114+
.quiz-actions {
115+
display: flex;
116+
gap: 0.5rem;
117+
margin-top: 0.75rem;
118+
flex-wrap: wrap;
119+
}
120+
.quiz-next-btn {
121+
padding: 0.5rem 1.25rem;
122+
border-radius: 8px;
123+
border: 1px solid var(--color-primary);
124+
background: var(--color-primary);
125+
color: #ffffff;
126+
font-family: var(--font-heading);
127+
font-size: 0.82rem;
128+
font-weight: 600;
129+
cursor: pointer;
130+
transition: all 0.2s ease;
131+
min-height: 36px;
132+
}
133+
.quiz-next-btn:hover {
134+
box-shadow: 0 0 12px var(--color-glow-primary);
135+
transform: translateY(-1px);
136+
}
137+
.quiz-learn-btn {
138+
padding: 0.4rem 0.75rem;
139+
border-radius: 6px;
140+
border: 1px solid var(--color-border);
141+
background: var(--color-surface-raised);
142+
color: var(--color-text-muted);
143+
font-family: var(--font-body);
144+
font-size: 0.75rem;
145+
cursor: pointer;
146+
transition: all 0.2s ease;
147+
text-decoration: none;
148+
display: inline-flex;
149+
align-items: center;
150+
gap: 0.35rem;
151+
min-height: 32px;
152+
}
153+
.quiz-learn-btn:hover {
154+
border-color: var(--color-secondary);
155+
color: var(--color-secondary);
156+
box-shadow: 0 0 8px var(--color-glow-secondary);
157+
}
113158
.katex { font-size: 1.05em; }
114159
`;
115160
document.head.appendChild(style);
@@ -128,6 +173,11 @@ export function init(container) {
128173
<button class="quiz-option" data-key="D" aria-label="Option D"></button>
129174
</div>
130175
<div class="quiz-feedback" aria-live="assertive"></div>
176+
<div class="quiz-actions" hidden>
177+
<button class="quiz-next-btn" aria-label="Next question">Next <i class="fa-solid fa-arrow-right" style="margin-left:0.3rem;font-size:0.75rem"></i></button>
178+
<a class="quiz-learn-btn" target="_blank" rel="noopener" data-learn="wikipedia" hidden><i class="fa-brands fa-wikipedia-w"></i> Wikipedia</a>
179+
<a class="quiz-learn-btn" target="_blank" rel="noopener" data-learn="khan" hidden><i class="fa-solid fa-graduation-cap"></i> Khan Academy</a>
180+
</div>
131181
<div class="quiz-meta"></div>
132182
</div>
133183
`;
@@ -138,6 +188,10 @@ export function init(container) {
138188
instruction: container.querySelector('.quiz-instruction'),
139189
options: container.querySelectorAll('.quiz-option'),
140190
feedback: container.querySelector('.quiz-feedback'),
191+
actions: container.querySelector('.quiz-actions'),
192+
nextBtn: container.querySelector('.quiz-next-btn'),
193+
wikiBtn: container.querySelector('[data-learn="wikipedia"]'),
194+
khanBtn: container.querySelector('[data-learn="khan"]'),
141195
meta: container.querySelector('.quiz-meta')
142196
};
143197

@@ -146,6 +200,12 @@ export function init(container) {
146200
btn.addEventListener('click', () => handleOptionClick(btn.dataset.key));
147201
});
148202

203+
if (uiElements.nextBtn) {
204+
uiElements.nextBtn.addEventListener('click', () => {
205+
if (nextCallback) nextCallback();
206+
});
207+
}
208+
149209
document.addEventListener('keydown', handleKeyDown);
150210

151211
const resizeHandle = container.querySelector('.resize-handle');
@@ -174,7 +234,15 @@ export function init(container) {
174234

175235
function handleKeyDown(e) {
176236
if (!currentQuestion || !uiElements.options) return;
177-
if (uiElements.options[0].disabled) return;
237+
238+
// If options are disabled (already answered), Enter/N/Space advances to next
239+
if (uiElements.options[0].disabled) {
240+
if (e.key === 'Enter' || e.key === 'n' || e.key === 'N' || e.key === ' ') {
241+
e.preventDefault();
242+
if (nextCallback) nextCallback();
243+
}
244+
return;
245+
}
178246

179247
const key = e.key.toUpperCase();
180248
if (['1', 'A'].includes(key)) handleOptionClick('A');
@@ -218,7 +286,12 @@ export function showQuestion(question) {
218286
}
219287

220288
if (uiElements.meta) {
221-
uiElements.meta.innerHTML = '';
289+
uiElements.meta.textContent = '';
290+
}
291+
292+
// Hide Next/Learn more buttons until answer is given
293+
if (uiElements.actions) {
294+
uiElements.actions.hidden = true;
222295
}
223296
}
224297

@@ -256,20 +329,56 @@ function handleOptionClick(selectedKey) {
256329
if (uiElements.meta && currentQuestion.source_article) {
257330
const article = currentQuestion.source_article;
258331
const url = `https://en.wikipedia.org/wiki/${encodeURIComponent(article)}`;
259-
uiElements.meta.innerHTML = `Source: <a href="${url}" target="_blank">${article}</a>`;
332+
uiElements.meta.textContent = '';
333+
const sourceLabel = document.createTextNode('Source: ');
334+
const sourceLink = document.createElement('a');
335+
sourceLink.href = url;
336+
sourceLink.target = '_blank';
337+
sourceLink.textContent = article;
338+
uiElements.meta.appendChild(sourceLabel);
339+
uiElements.meta.appendChild(sourceLink);
260340
}
261341

342+
// Show Next button and Learn more links instead of auto-advancing
343+
if (uiElements.actions) {
344+
uiElements.actions.hidden = false;
345+
346+
// Show Wikipedia link if we have a source article
347+
if (uiElements.wikiBtn && currentQuestion.source_article) {
348+
const wikiUrl = `https://en.wikipedia.org/wiki/${encodeURIComponent(currentQuestion.source_article)}`;
349+
uiElements.wikiBtn.href = wikiUrl;
350+
uiElements.wikiBtn.hidden = false;
351+
} else if (uiElements.wikiBtn) {
352+
uiElements.wikiBtn.hidden = true;
353+
}
354+
355+
// Show Khan Academy search link based on question concepts or article
356+
if (uiElements.khanBtn) {
357+
const searchTerm = currentQuestion.source_article || '';
358+
if (searchTerm) {
359+
uiElements.khanBtn.href = `https://www.khanacademy.org/search?referer=%2F&page_search_query=${encodeURIComponent(searchTerm)}`;
360+
uiElements.khanBtn.hidden = false;
361+
} else {
362+
uiElements.khanBtn.hidden = true;
363+
}
364+
}
365+
}
366+
367+
// Fire the answer callback immediately (records the response and updates estimator)
368+
// but do NOT auto-advance to next question — user clicks "Next" when ready
262369
if (answerCallback) {
263-
setTimeout(() => {
264-
answerCallback(selectedKey, currentQuestion);
265-
}, 800);
370+
answerCallback(selectedKey, currentQuestion);
266371
}
267372
}
268373

269374
export function onAnswer(callback) {
270375
answerCallback = callback;
271376
}
272377

378+
export function onNext(callback) {
379+
nextCallback = callback;
380+
}
381+
273382
/**
274383
* Replaces $...$ with KaTeX rendered HTML.
275384
* Heuristic: if content between $ signs contains only numbers/punctuation, treat as currency.

0 commit comments

Comments
 (0)