Refactor hot-spots (012): foundational dedup + quiz-overlay XSS fix + answer-reveal/instructor-auth consolidation

specs/012-code-review/quickstart.md

@@ -73,6 +73,24 @@ For every extraction:
 | SC-005 (tests green, bundle OK) | full DoD command block passes; `size-check` under 40KB |
 | SC-006 (independently shippable) | each slice merges on its own with green CI |
 
+## Baseline (regression budget) — recorded T001/T002
+
+Captured before refactor (2026-06-19):
+
+- **Tests**: 782 unit + 67 integration passing; typecheck/lint/format clean.
+- **Bundle**: 146.12 KB min / **36.71 KB gzip** (limit 40 KB; 3.29 KB headroom). This is the regression budget — no slice may exceed 40 KB gzip.
+- **Files >400 lines (SC-002 targets)**:
+  | File | Lines |
+  |------|-------|
+  | src/components/qd-login.ts | 983 |
+  | src/enhancers/quiz-table.ts | 807 |
+  | src/services/storage/indexeddb.ts | 759 |
+  | src/enhancers/analysis-table.ts | 637 |
+  | src/components/qd-migration-dialog.ts | 519 |
+  | src/init/bootstrap.ts | 490 |
+  | src/services/session.ts | 443 |
+  | src/types/contracts.ts | 411 (FROZEN — out of scope) |
+
 ## Out of scope (do not touch)
 - `src/types/contracts.ts` (frozen).
 - `home-badges` → Lit conversion (breaks progressive enhancement).

specs/012-code-review/tasks.md

@@ -30,8 +30,8 @@ Behavior-preserving, test-gated, one slice at a time. The **only** sanctioned be
 
 **Purpose**: Confirm the baseline is green so every later slice can be verified against it.
 
-- [ ] T001 Run the full Definition of Done baseline (`npm run typecheck && npm run lint && npm run test:unit && npm run test:integration && npm run format:check && npm run build && npm run size-check`) and record the current bundle size as the regression budget in `specs/012-code-review/quickstart.md` notes
-- [ ] T002 [P] Capture current file line counts for all in-scope modules (`wc -l` for the 8 target files) as the before/after baseline for SC-002
+- [X] T001 Run the full Definition of Done baseline (`npm run typecheck && npm run lint && npm run test:unit && npm run test:integration && npm run format:check && npm run build && npm run size-check`) and record the current bundle size as the regression budget in `specs/012-code-review/quickstart.md` notes
+- [X] T002 [P] Capture current file line counts for all in-scope modules (`wc -l` for the 8 target files) as the before/after baseline for SC-002
 
 ---
 
@@ -41,12 +41,12 @@ Behavior-preserving, test-gated, one slice at a time. The **only** sanctioned be
 
 **⚠️ CRITICAL**: Complete before US2/US3/US4 extraction work begins.
 
-- [ ] T003 [P] Add `INSTRUCTOR_SHOW_ANSWERS` constant to `STORAGE_KEYS` in `src/utils/storage-helpers.ts` (or the existing keys module) and replace the `'qd/instructor/showAnswers'` magic string in `src/services/session.ts`, `src/init/bootstrap.ts`, and `src/init/event-coordinator.ts`
-- [ ] T004 [P] Create `src/utils/page-id.ts` exporting `getPageIdFromUrl(url?)` and replace the inline pathname→filename→strip-`.html` parses in `src/init/bootstrap.ts` (×2) and `src/init/event-coordinator.ts`
-- [ ] T005 [P] Add unit test `tests/unit/page-id.test.ts` for `getPageIdFromUrl` (root, nested, query string, hash, missing `.html`)
-- [ ] T006 [P] Add `clearBadges(link)` helper in `src/enhancers/home-badges.ts` and replace the three duplicated badge-stripping loops
-- [ ] T007 [P] Add `createEmptyStudentRecord(session)` to `src/services/storage-service.ts` and replace the two duplicated new-record literals in `loadStudentRecord`
-- [ ] T008 Consolidate shared styles (`button.primary`, `.error-message`, `.button-row`, spinner, modal-overlay) into `src/components/qd-instructor/shared-styles.ts` and import them in `qd-migration-dialog.ts`, `qd-pin-create.ts`, and `qd-login.ts`
+- [X] T003 [P] Add `INSTRUCTOR_SHOW_ANSWERS` constant to `STORAGE_KEYS` in `src/utils/storage-helpers.ts` (or the existing keys module) and replace the `'qd/instructor/showAnswers'` magic string in `src/services/session.ts`, `src/init/bootstrap.ts`, and `src/init/event-coordinator.ts`
+- [X] T004 [P] Create `src/utils/page-id.ts` exporting `getPageIdFromUrl(url?)` and replace the inline pathname→filename→strip-`.html` parses in `src/init/bootstrap.ts` (×2) and `src/init/event-coordinator.ts`
+- [X] T005 [P] Add unit test `tests/unit/page-id.test.ts` for `getPageIdFromUrl` (root, nested, query string, hash, missing `.html`)
+- [X] T006 [P] Add `clearBadges(link)` helper in `src/enhancers/home-badges.ts` and replace the three duplicated badge-stripping loops
+- [X] T007 [P] Add `createEmptyStudentRecord(session)` to `src/services/storage-service.ts` and replace the two duplicated new-record literals in `loadStudentRecord`
+- [X] T008 Consolidate shared styles (`button.primary`, `.error-message`, `.button-row`, spinner, modal-overlay) into `src/components/qd-instructor/shared-styles.ts` and import them in `qd-migration-dialog.ts`, `qd-pin-create.ts`, and `qd-login.ts`
 
 **Checkpoint**: Shared helpers/constants/styles in place; no observable behavior change; all tests green.
 
@@ -60,9 +60,9 @@ Behavior-preserving, test-gated, one slice at a time. The **only** sanctioned be
 
 > This story is the primary deliverable and is already produced. Tasks below verify it against the spec's acceptance criteria.
 
-- [ ] T009 [US1] Verify `specs/012-code-review/code-review-report.md` lists every `src/**` file >400 lines with line count, severity, and a decomposition recommendation (FR-001 / SC-001), reconciling against the T002 baseline
-- [ ] T010 [P] [US1] Verify the report addresses all five hot-spot criteria (UI/logic coupling, deep nesting, extractable components, Lit candidates, oversized files) each with specific file/line references (FR-002)
-- [ ] T011 [P] [US1] Verify the report includes a recommended execution order ranked lowest-risk → highest-payoff (FR-003)
+- [X] T009 [US1] Verify `specs/012-code-review/code-review-report.md` lists every `src/**` file >400 lines with line count, severity, and a decomposition recommendation (FR-001 / SC-001), reconciling against the T002 baseline
+- [X] T010 [P] [US1] Verify the report addresses all five hot-spot criteria (UI/logic coupling, deep nesting, extractable components, Lit candidates, oversized files) each with specific file/line references (FR-002)
+- [X] T011 [P] [US1] Verify the report includes a recommended execution order ranked lowest-risk → highest-payoff (FR-003)
 
 **Checkpoint**: Report validated; MVP deliverable complete.
 
@@ -76,18 +76,18 @@ Behavior-preserving, test-gated, one slice at a time. The **only** sanctioned be
 
 ### Tests for User Story 2 (write FIRST, confirm red/green) ⚠️
 
-- [ ] T012 [P] [US2] Characterization test `tests/integration/quiz-instructor-overlay.test.ts` asserting current rendered student-answer overlay content and structure (baseline before XSS fix)
-- [ ] T013 [P] [US2] Add XSS regression test in `tests/integration/quiz-instructor-overlay.test.ts`: a student answer/name containing `<script>`/HTML must render as literal text, not markup (FR-004, SC-003)
-- [ ] T014 [P] [US2] Characterization test `tests/integration/instructor-answer-reveal.test.ts` capturing the current reveal behavior (columns unhidden, correct answers re-injected) for both initial-load and post-login paths (FR-005)
+- [X] T012 [P] [US2] Characterization test `tests/integration/quiz-instructor-overlay.test.ts` asserting current rendered student-answer overlay content and structure (baseline before XSS fix)
+- [X] T013 [P] [US2] Add XSS regression test in `tests/integration/quiz-instructor-overlay.test.ts`: a student answer/name containing `<script>`/HTML must render as literal text, not markup (FR-004, SC-003)
+- [X] T014 [P] [US2] Characterization test `tests/integration/instructor-answer-reveal.test.ts` capturing the current reveal behavior (columns unhidden, correct answers re-injected) for both initial-load and post-login paths (FR-005)
 - [ ] T015 [P] [US2] Characterization test `tests/unit/auth-service.test.ts` covering the current student login outcomes: success, new student, lockout, bad PIN, needs-migration, and retry-after-migration (FR-006)
-- [ ] T016 [P] [US2] Unit test `tests/unit/instructor-auth.test.ts` for SHA-256 + 12-char-truncation hashing/verification (FR-008)
+- [X] T016 [P] [US2] Unit test `tests/unit/instructor-auth.test.ts` for SHA-256 + 12-char-truncation hashing/verification (FR-008)
 
 ### Implementation for User Story 2
 
-- [ ] T017 [P] [US2] Create `src/services/auth/instructor-auth.ts` (`hashPassword`, `verifyInstructorPassword`) per contracts; route `src/components/qd-login.ts` and `src/components/qd-migration-dialog.ts` through it, deleting the duplicated crypto
+- [X] T017 [P] [US2] Create `src/services/auth/instructor-auth.ts` (`hashPassword`, `verifyInstructorPassword`) per contracts; route `src/components/qd-login.ts` and `src/components/qd-migration-dialog.ts` through it, deleting the duplicated crypto
 - [ ] T018 [US2] Route all DOM config reads (DB name, title, hash) in `src/components/qd-login.ts`, `src/components/qd-pin-reset-dialog.ts`, and `src/components/qd-migration-dialog.ts` through `src/config/dom-config-reader.ts` (`readDOMConfig`/`CONFIG_IDS`), removing inline `document.getElementById(...)` reads (FR-008)
-- [ ] T019 [US2] Create `src/enhancers/instructor-answer-reveal.ts` exporting `revealInstructorAnswers`/`hideInstructorAnswers`; replace the duplicated instructor branches in `src/init/bootstrap.ts` (`revealQuizAnswersForInstructor`) and `src/init/event-coordinator.ts` (`upgradeTablesAfterLogin`) with calls to it (FR-005, SC-004)
-- [ ] T020 [US2] Fix the `innerHTML` XSS in the quiz instructor overlay in `src/enhancers/quiz-table.ts` by rendering student-supplied fields via `textContent`/element construction (FR-004, SC-003) — make T013 pass
+- [X] T019 [US2] Create `src/enhancers/instructor-answer-reveal.ts` exporting `revealInstructorAnswers`/`hideInstructorAnswers`; replace the duplicated instructor branches in `src/init/bootstrap.ts` (`revealQuizAnswersForInstructor`) and `src/init/event-coordinator.ts` (`upgradeTablesAfterLogin`) with calls to it (FR-005, SC-004)
+- [X] T020 [US2] Fix the `innerHTML` XSS in the quiz instructor overlay in `src/enhancers/quiz-table.ts` by rendering student-supplied fields via `textContent`/element construction (FR-004, SC-003) — make T013 pass
 - [ ] T021 [US2] Create `src/services/auth/auth-service.ts` with `loginStudent`/`retryAfterMigration` returning the result union per contracts; consolidate the duplicated success path so `handleStudentLogin` and `retryLoginAfterMigration` in `src/components/qd-login.ts` both delegate, deleting the ~100-line duplicate (FR-006, SC-004) — make T015 pass
 
 **Checkpoint**: No student data rendered via `innerHTML`; answer-reveal and login-success logic each exist in exactly one place; instructor-auth and config reads de-duplicated.

src/components/qd-instructor/qd-instructor.ts

@@ -8,7 +8,7 @@ import { customElement, state } from 'lit/decorators.js';
 import { sharedStyles } from './shared-styles.js';
 import type { StudentRecord, SessionData } from '../../types/contracts.js';
 import { STORAGE_KEYS } from '../../types/contracts.js';
-import { getJSON } from '../../utils/storage-helpers.js';
+import { getJSON, INSTRUCTOR_SHOW_ANSWERS_KEY } from '../../utils/storage-helpers.js';
 import { SessionService } from '../../services/session.js';
 import { getStorageService } from '../../services/storage-service.js';
 import './qd-instructor-unlock.js';
@@ -77,7 +77,7 @@ export class QdInstructor extends LitElement {
     }
 
     // Restore toggle state from sessionStorage
-    const savedState = sessionStorage.getItem('qd/instructor/showAnswers');
+    const savedState = sessionStorage.getItem(INSTRUCTOR_SHOW_ANSWERS_KEY);
     if (savedState !== null) {
       this.showStudentAnswers = savedState === 'true';
 
@@ -301,7 +301,7 @@ export class QdInstructor extends LitElement {
     );
 
     // Persist toggle state in sessionStorage
-    sessionStorage.setItem('qd/instructor/showAnswers', String(this.showStudentAnswers));
+    sessionStorage.setItem(INSTRUCTOR_SHOW_ANSWERS_KEY, String(this.showStudentAnswers));
   };
 
   private handleHelpOpen = (): void => {

src/components/qd-login.ts

@@ -28,6 +28,7 @@ import { CONFIG_IDS } from '../config/dom-config-reader.js';
 import { getStorageAdapter } from '../services/storage/indexeddb.js';
 import { needsMigration, hasPinSet, completePinSetup } from '../services/storage/migration.js';
 import { verifyPin, hashPin } from '../services/auth/pin-service.js';
+import { hashPassword, getExpectedInstructorHash } from '../services/auth/instructor-auth.js';
 import {
   checkLockout,
   recordFailedAttempt,
@@ -899,42 +900,19 @@ export class QdLogin extends LitElement {
     this.instructorError = '';
   }
 
-  /**
-   * Hash password using SHA-256
-   */
-  private async hashPassword(password: string): Promise<string> {
-    const encoder = new TextEncoder();
-    const data = encoder.encode(password);
-    const hashBuffer = await crypto.subtle.digest('SHA-256', data);
-    const hashArray = Array.from(new Uint8Array(hashBuffer));
-    // Return first 12 characters for author-friendly Oxygen dialogs
-    return hashArray
-      .map((b) => b.toString(16).padStart(2, '0'))
-      .join('')
-      .substring(0, 12);
-  }
-
-  /**
-   * Get expected password hash from hidden element
-   */
-  private getExpectedHash(): string {
-    const hashElement = document.getElementById(CONFIG_IDS.instructorHash);
-    return hashElement?.textContent?.trim() || '';
-  }
-
   /**
    * Handle instructor login with password
    */
   private async handleInstructorLogin(password: string) {
     try {
-      const passwordHash = await this.hashPassword(password);
-      const expectedHash = this.getExpectedHash();
+      const expectedHash = getExpectedInstructorHash();
 
       if (!expectedHash) {
         this.instructorError = 'Instructor password not configured';
         return;
       }
 
+      const passwordHash = await hashPassword(password);
       if (passwordHash !== expectedHash) {
         this.instructorError = 'Incorrect password';
         // TODO: Implement rate limiting (5 attempts per 60 seconds)