Refactor architectural hot-spots for maintainability (012 — complete)

specs/012-code-review/code-review-report.md

@@ -149,3 +149,27 @@ Files 400–340 lines that are *structurally healthy* (no split required): `qd-s
 7. **Extract reusable Lit components**: `<qd-student-answers>` / `<qd-student-entries>`, `qd-student-table`, `qd-spinner`, `qd-lockout-banner`; adopt `qd-modal` base in `qd-pin-create`.
 
 Each step should keep tests green per the Definition of Done (typecheck, lint, unit/integration, format, build, <40KB bundle). Refactors must preserve behavior — TDD: characterize current behavior with tests before moving logic.
+
+---
+
+## 7. Resolution (post-refactor — final state)
+
+All findings above have been implemented behavior-preservingly (the only sanctioned behavior change is the quiz instructor-overlay `innerHTML` → `textContent` XSS fix). Final line counts for the former hot-spots:
+
+| File | Before | After | Outcome |
+|------|-------:|------:|---------|
+| `src/components/qd-login.ts` | 983 | 395 | Presentational view over `AuthService`; styles → `qd-login.styles.ts` |
+| `src/enhancers/quiz-table.ts` | 807 | 399 | Split → `quiz-table-columns`, `quiz-input-factory`, `quiz-answer-persistence`, `quiz-instructor-overlay` |
+| `src/services/storage/indexeddb.ts` | 759 | 308 | Thin coordinator → `idb-connection`, `idb-helpers`, `idb-codec`, `backup-repository`, `audit-log-repository` |
+| `src/enhancers/analysis-table.ts` | 637 | 301 | Split → `analysis-display`, `analysis-persistence`, `analysis-instructor-overlay` |
+| `src/components/qd-migration-dialog.ts` | 519 | 344 | Styles extracted; uses shared `instructor-auth` + `qd-spinner` |
+| `src/init/bootstrap.ts` | 490 | 321 | CSS literal → `global-styles.ts`; parameterized table enhancement |
+| `src/services/session.ts` | 443 | 269 | Cache math → `session-cache.ts` |
+| `src/init/event-coordinator.ts` | 339 | 216 | Reduced to a pure event router |
+
+**Success criteria**:
+- **SC-002**: `find src -name '*.ts' | xargs wc -l | awk '$1>400'` returns only the frozen `src/types/contracts.ts` (411). ✅
+- **SC-003**: no enhancer overlay renders student data via `innerHTML`; quiz/analysis overlays are backed by the auto-escaped `<qd-student-answers>` / `<qd-student-entries>` Lit components. ✅
+- **SC-005**: full Definition of Done green (831 unit + 84 integration tests); IIFE bundle 37.6 KB gzip (≤40 KB limit, +0.9 KB vs the 36.7 KB baseline from the four new reusable Lit components). ✅
+
+Reusable components added: `<qd-student-answers>`, `<qd-student-entries>`, `<qd-student-table>`, `<qd-spinner>`. New shared helpers: `session-state.ts`, `persist-and-notify.ts`, `auth-service.ts`, `instructor-auth.ts`, `pin-reset-service.ts`, `page-id.ts`.

src/components/qd-instructor-login.ts

@@ -0,0 +1,141 @@
+/**
+ * Instructor Login Component
+ *
+ * Encapsulates the instructor authentication flow: the "Instructor" button, the
+ * password modal, and verification. Extracted from `qd-login` so the login
+ * component only owns the student flow.
+ *
+ * @element qd-instructor-login
+ * @fires {CustomEvent<{serviceId:'INSTRUCTOR', name:'Instructor', release:string, role:'instructor'}>} qd:login - On successful instructor auth
+ */
+
+import { LitElement, html, css } from 'lit';
+import { customElement, state, property } from 'lit/decorators.js';
+import { STORAGE_KEYS } from '../types/contracts.js';
+import { SessionService } from '../services/session.js';
+import { readRelease } from '../config/dom-config-reader.js';
+import {
+  getExpectedInstructorHash,
+  verifyInstructorPassword,
+} from '../services/auth/instructor-auth.js';
+import './qd-password-modal.js';
+
+/**
+ * Instructor login button + password modal.
+ */
+@customElement('qd-instructor-login')
+export class QdInstructorLogin extends LitElement {
+  /** Disable the button (e.g. while the parent form is submitting). */
+  @property({ type: Boolean })
+  disabled = false;
+
+  @state()
+  private showModal = false;
+
+  @state()
+  private error = '';
+
+  static styles = css`
+    button {
+      padding: 6px 12px;
+      border: none;
+      border-radius: 4px;
+      font-size: 11px;
+      font-weight: 500;
+      cursor: pointer;
+      transition: all 0.2s;
+      white-space: nowrap;
+      background: #6c757d;
+      color: white;
+    }
+
+    button:hover:not(:disabled) {
+      background: #5a6268;
+    }
+
+    button:disabled {
+      cursor: not-allowed;
+      opacity: 0.7;
+    }
+  `;
+
+  render() {
+    return html`
+      <button type="button" class="instructor-btn" @click=${this.open} ?disabled=${this.disabled}>
+        Instructor
+      </button>
+
+      <qd-password-modal
+        .open=${this.showModal}
+        title="Instructor Login"
+        .error=${this.error}
+        @qd:password-submit=${this.handleSubmit}
+        @close=${this.handleClose}
+      ></qd-password-modal>
+    `;
+  }
+
+  private open = (): void => {
+    this.showModal = true;
+    this.error = '';
+  };
+
+  private handleClose = (): void => {
+    this.showModal = false;
+    this.error = '';
+  };
+
+  private handleSubmit = (e: CustomEvent<{ password: string }>): void => {
+    void this.login(e.detail.password);
+  };
+
+  /**
+   * Verify the password and, on success, create the instructor session and
+   * emit `qd:login`.
+   */
+  private async login(password: string): Promise<void> {
+    const expectedHash = getExpectedInstructorHash();
+    if (!expectedHash) {
+      this.error = 'Instructor password not configured';
+      return;
+    }
+
+    try {
+      if (!(await verifyInstructorPassword(password))) {
+        this.error = 'Incorrect password';
+        return;
+      }
+
+      const release = readRelease();
+
+      const sessionService = new SessionService();
+      sessionService.createSession('INSTRUCTOR', 'Instructor', release || '');
+      sessionStorage.setItem(STORAGE_KEYS.INSTRUCTOR, 'true');
+
+      this.dispatchEvent(
+        new CustomEvent('qd:login', {
+          detail: {
+            serviceId: 'INSTRUCTOR',
+            name: 'Instructor',
+            release: release || '',
+            role: 'instructor',
+          },
+          bubbles: true,
+          composed: true,
+        }),
+      );
+
+      this.showModal = false;
+      this.error = '';
+    } catch (err) {
+      this.error = 'Login failed. Please try again.';
+      console.error('Instructor login error:', err);
+    }
+  }
+}
+
+declare global {
+  interface HTMLElementTagNameMap {
+    'qd-instructor-login': QdInstructorLogin;
+  }
+}

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

@@ -9,6 +9,7 @@ import { sharedStyles } from './shared-styles.js';
 import type { StudentRecord, SessionData } from '../../types/contracts.js';
 import { STORAGE_KEYS } from '../../types/contracts.js';
 import { getJSON, INSTRUCTOR_SHOW_ANSWERS_KEY } from '../../utils/storage-helpers.js';
+import { isInstructor } from '../../utils/session-state.js';
 import { SessionService } from '../../services/session.js';
 import { getStorageService } from '../../services/storage-service.js';
 import './qd-instructor-unlock.js';
@@ -69,11 +70,11 @@ export class QdInstructor extends LitElement {
     this.updateVisibility();
 
     // Auto-unlock if instructor is already logged in
-    const isInstructor = sessionStorage.getItem(STORAGE_KEYS.INSTRUCTOR) === 'true';
-    if (isInstructor) {
+    const instructorActive = isInstructor();
+    if (instructorActive) {
       this.unlock();
       // Load students data for export button
-      void this.loadStudents();
+      void this.refreshStudents();
     }
 
     // Restore toggle state from sessionStorage
@@ -82,7 +83,7 @@ export class QdInstructor extends LitElement {
       this.showStudentAnswers = savedState === 'true';
 
       // If toggle was enabled and instructor is logged in, dispatch event to show answers
-      if (this.showStudentAnswers && isInstructor) {
+      if (this.showStudentAnswers && instructorActive) {
         // Dispatch after tables are enhanced (use setTimeout to defer)
         setTimeout(() => {
           this.dispatchEvent(
@@ -109,12 +110,7 @@ export class QdInstructor extends LitElement {
    * Update visibility based on instructor session state
    */
   private updateVisibility(): void {
-    const isInstructor = sessionStorage.getItem(STORAGE_KEYS.INSTRUCTOR) === 'true';
-    if (isInstructor) {
-      this.setAttribute('data-show', '');
-    } else {
-      this.removeAttribute('data-show');
-    }
+    this.toggleAttribute('data-show', isInstructor());
   }
 
   private handleLoginEvent = (event: Event): void => {
@@ -127,7 +123,7 @@ export class QdInstructor extends LitElement {
     if (role === 'instructor') {
       this.unlock();
       // Load students data for export button
-      void this.loadStudents();
+      void this.refreshStudents();
     }
   };
 
@@ -144,16 +140,18 @@ export class QdInstructor extends LitElement {
   }
 
   /**
-   * Load students from storage for current release
+   * Load all students for the current release into `this.students`.
+   *
+   * The single source for loading students; every panel/handler that needs a
+   * fresh list calls this instead of duplicating the load/try-catch.
    */
-  private async loadStudents(): Promise<void> {
+  private async refreshStudents(): Promise<void> {
     const session = getJSON<SessionData>(STORAGE_KEYS.SESSION);
     if (!session) return;
 
     try {
       const storageService = getStorageService();
-      const students = await storageService.getStudentsByRelease(session.release);
-      this.students = students;
+      this.students = await storageService.getStudentsByRelease(session.release);
     } catch (err) {
       console.error('Failed to load students:', err);
       this.students = [];
@@ -178,18 +176,7 @@ export class QdInstructor extends LitElement {
 
   private handleResetPins = async (): Promise<void> => {
     // Load all students for current release before showing reset dialog
-    const session = getJSON<SessionData>(STORAGE_KEYS.SESSION);
-    if (!session) return;
-
-    try {
-      const storageService = getStorageService();
-      const students = await storageService.getStudentsByRelease(session.release);
-      this.students = students;
-    } catch (err) {
-      console.error('Failed to load students:', err);
-      this.students = [];
-    }
-
+    await this.refreshStudents();
     this.showPinReset = true;
   };
 
@@ -220,18 +207,7 @@ export class QdInstructor extends LitElement {
 
   private handleViewScores = async (): Promise<void> => {
     // Load all students for current release before showing scores
-    const session = getJSON<SessionData>(STORAGE_KEYS.SESSION);
-    if (!session) return;
-
-    try {
-      const storageService = getStorageService();
-      const students = await storageService.getStudentsByRelease(session.release);
-      this.students = students;
-    } catch (err) {
-      console.error('Failed to load students:', err);
-      this.students = [];
-    }
-
+    await this.refreshStudents();
     this.showScores = true;
   };
 
@@ -276,16 +252,7 @@ export class QdInstructor extends LitElement {
 
     // FR-004: Load student data in fresh session when toggle is enabled
     if (this.showStudentAnswers && this.students.length === 0) {
-      const session = getJSON<SessionData>(STORAGE_KEYS.SESSION);
-      if (session) {
-        try {
-          const storageService = getStorageService();
-          const students = await storageService.getStudentsByRelease(session.release);
-          this.students = students;
-        } catch (err) {
-          console.error('Failed to load students for toggle:', err);
-        }
-      }
+      await this.refreshStudents();
     }
 
     // Emit event to notify table enhancers