From 4cb716fe0de0cfe157579df5bb72046be8bbfaeb Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Wed, 17 Jun 2026 06:09:55 +0000
Subject: [PATCH 1/3] Add code review report and spec for refactoring hot-spots

Conduct a code review of large/complex modules and produce a prioritized
report of refactoring recommendations covering: tight UI/business-logic
coupling, deeply nested logic, extractable child components, Lit-component
candidates, and files over 400 lines.

Deliverables under specs/012-code-review/:
- code-review-report.md: prioritized hot-spot findings with file/line refs
- spec.md: refactoring feature spec derived from the report
- checklists/requirements.md: spec quality checklist

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvXAFvUpR6jxvQYTofu4eG
---
 .../checklists/requirements.md                |  38 +++++
 specs/012-code-review/code-review-report.md   | 151 ++++++++++++++++++
 specs/012-code-review/spec.md                 | 135 ++++++++++++++++
 3 files changed, 324 insertions(+)
 create mode 100644 specs/012-code-review/checklists/requirements.md
 create mode 100644 specs/012-code-review/code-review-report.md
 create mode 100644 specs/012-code-review/spec.md

diff --git a/specs/012-code-review/checklists/requirements.md b/specs/012-code-review/checklists/requirements.md
new file mode 100644
index 0000000..502e5b2
--- /dev/null
+++ b/specs/012-code-review/checklists/requirements.md
@@ -0,0 +1,38 @@
+# Specification Quality Checklist: Refactor Architectural Hot-Spots for Maintainability
+
+**Purpose**: Validate specification completeness and quality before proceeding to planning
+**Created**: 2026-06-17
+**Feature**: [spec.md](../spec.md)
+
+## Content Quality
+
+- [x] No implementation details (languages, frameworks, APIs)
+- [x] Focused on user value and business needs
+- [x] Written for non-technical stakeholders
+- [x] All mandatory sections completed
+
+## Requirement Completeness
+
+- [x] No [NEEDS CLARIFICATION] markers remain
+- [x] Requirements are testable and unambiguous
+- [x] Success criteria are measurable
+- [x] Success criteria are technology-agnostic (no implementation details)
+- [x] All acceptance scenarios are defined
+- [x] Edge cases are identified
+- [x] Scope is clearly bounded
+- [x] Dependencies and assumptions identified
+
+## Feature Readiness
+
+- [x] All functional requirements have clear acceptance criteria
+- [x] User scenarios cover primary flows
+- [x] Feature meets measurable outcomes defined in Success Criteria
+- [x] No implementation details leak into specification
+
+## Notes
+
+- This is an internal engineering-maintainability feature; "stakeholders" are maintainers, so some technical
+  vocabulary (module, component) is unavoidable but kept conceptual in the spec. Concrete file/line detail and
+  framework-specific recommendations live in the companion `code-review-report.md`, not in `spec.md`.
+- Success criteria SC-002/SC-003/SC-004 are measurable and verifiable without prescribing implementation.
+- All items pass. Spec is ready for `/speckit.clarify` (optional) or `/speckit.plan`.
diff --git a/specs/012-code-review/code-review-report.md b/specs/012-code-review/code-review-report.md
new file mode 100644
index 0000000..d52adb9
--- /dev/null
+++ b/specs/012-code-review/code-review-report.md
@@ -0,0 +1,151 @@
+# Code Review Report: Refactoring Hot-Spots
+
+**Created**: 2026-06-17
+**Branch**: `claude/speckit-code-review-8dd0zw`
+**Scope**: `src/**` — identify modules/blocks where (a) UI and business logic are too tightly intertwined, (b) business logic is several layers deep, (c) child components could be refactored out, (d) UI elements could become Lit components, (e) files exceed 400 lines.
+
+## How to read this report
+
+Each hot-spot is rated **HIGH / MEDIUM / LOW** by refactoring payoff (size + complexity reduction, risk removed, duplication eliminated). Line references are point-in-time against the current `main`-derived branch and should be re-confirmed before editing. The report is descriptive; the companion `spec.md` turns the prioritized findings into requirements.
+
+---
+
+## 1. Files over 400 lines (size hot-spots)
+
+| File | Lines | Severity | Primary problem |
+|------|-------|----------|-----------------|
+| `src/components/qd-login.ts` | 983 | HIGH | Component is also the entire auth/PIN/migration/instructor pipeline; ~100-line duplicated method |
+| `src/enhancers/quiz-table.ts` | 807 | HIGH | Parsing + persistence + DOM + instructor overlay fused; `innerHTML` XSS risk |
+| `src/services/storage/indexeddb.ts` | 759 | HIGH | One class doing connection, migration, encryption, backups, audit log; ~150 lines of callback boilerplate |
+| `src/enhancers/analysis-table.ts` | 637 | HIGH | Same fusion as quiz-table; pure logic + inline-CSS rendering embedded |
+| `src/components/qd-migration-dialog.ts` | 519 | MEDIUM | Duplicated password hashing; large inline styles |
+| `src/init/bootstrap.ts` | 490 | HIGH | "God bootstrap": CSS literal + orchestration + duplicated instructor answer-reveal business rules |
+| `src/services/session.ts` | 443 | MEDIUM | Two concerns glued together (session lifecycle + pure cache math) |
+| `src/types/contracts.ts` | 411 | LOW | Frozen contracts file; size expected, leave as-is |
+
+Files 400–340 lines that are *structurally healthy* (no split required): `qd-status.ts` (354), `qd-instructor.ts` (380), `qd-pin-reset-dialog.ts` (389), `qd-pin-create.ts` (364), `validation.ts` (364).
+
+---
+
+## 2. HIGH-severity hot-spots
+
+### 2.1 `qd-login.ts` (983 lines) — UI fused with the entire auth pipeline
+
+- **Tight intertwining**: `handleStudentLogin` (L542–694) is a ~150-line event handler that performs form validation, DOM release extraction, lockout checks, raw config reads, storage init, student lookup, migration detection, PIN hashing/setup/verification, rate-limit recording, session creation, and 4 `CustomEvent` dispatches. This is a service masquerading as a UI handler.
+- **Duplication (largest in the file)**: `retryLoginAfterMigration` (L731–829) is a near-verbatim ~100-line copy of `handleStudentLogin`'s success path (compare L584–666 vs L746–821).
+- **Deep nesting**: `handleStudentLogin` reaches 4–5 levels (`try` → `if existingStudent` → migration/PIN branches → lockout check); mirrored in the retry method.
+- **Embedded crypto**: `hashPassword` (L905–915) + `getExpectedHash` (L920–923) embed SHA-256 + 12-char truncation in the component — duplicated verbatim in `qd-migration-dialog.ts` `validatePassword` (L319–339).
+- **Config bypass**: inline `document.getElementById(...).textContent.trim()` for DB-name/release appears 3+ times (L529–537, L573–579, L674–675, L740–741), bypassing the existing `readDOMConfig()` / `CONFIG_IDS` service.
+
+**Recommended extractions**
+1. `AuthService.loginStudent(...)` returning a discriminated union (`success | lockout | bad-pin | needs-migration`). Collapses both login paths into one, removes the ~100-line duplicate, flattens nesting. The component becomes a thin view mapping results → state.
+2. Shared `instructor-auth` helper for password hashing (also used by migration dialog).
+3. Route all config reads through `readDOMConfig()`.
+4. Optional `qd-lockout-banner` reusable component owning its own countdown interval (currently `lockoutInterval` / `startLockoutCountdown` L845–862).
+
+**Expected outcome**: file drops well below 400 lines and becomes a presentational component.
+
+### 2.2 `quiz-table.ts` (807 lines) — persistence + DOM + instructor overlay fused; XSS risk
+
+- **Security — `innerHTML` injection (highest priority)**: the student-answer overlay (L771–788, esp. L779–783) builds markup via `innerHTML` with unescaped student-controlled `sa.name` / `sa.answer`. This is an XSS risk and contrary to the spirit of Constitution VIII. The analysis enhancer does the equivalent correctly with `textContent` — the quiz path regressed.
+- **Tight intertwining**: `saveAnswer` (L465–558) is a ~90-line function doing session lookup, validation, record construction, IndexedDB load/mutate/save, cache rebuild + sessionStorage write, DOM cell selection + validation styling, and two event emissions.
+- **Long/nested function**: `enhanceInteractive` (L188–364) is 176 lines interleaving cache bootstrap, row iteration + input injection, instructor-listener wiring, and login/logout UI-reset logic (with an inline `resetUIState` closure L313–335).
+
+**Recommended extractions**
+- `<qd-student-answers>` Lit component for the overlay (auto-escapes, removes XSS).
+- `quiz-answer-persistence.ts` for `handleAnswerInput` + `saveAnswer` + validation styling.
+- `quiz-table-columns.ts` for the pure column show/hide helpers (`removeColgroup`/`hideAnswerColumn`/`showAnswerColumn`/`hideDetailColumn`, L580–662).
+- `quiz-input-factory.ts` for `createQuestionInput` (L378–420).
+- `quiz-instructor-overlay.ts` for show/hide student answers (L719–806).
+
+### 2.3 `analysis-table.ts` (637 lines) — same fusion + inline-CSS rendering
+
+- **Tight intertwining**: `saveCellData` (L284–363) hand-rolls get-or-create + mutate persistence that has no `updateRecordWithAnalysis` equivalent on `storage-service` (the quiz path has `updateRecordWithAnswer`).
+- **Pure logic in a DOM file**: `groupEntriesByCell` (L394–424) and `sortByTimestamp` (L432–438) are pure and already `export`ed — they belong in a `services/analysis-display.ts` (parallel to existing `services/answer-display.ts`).
+- **Inline CSS strings (Shadow-DOM violation)**: `createStudentEntriesDisplay` (L446–490) sets `element.style.cssText = 'color:#9ca3af;…'` with hard-coded hex colors (L454, L466, L475, L479, L487), contrary to the "Shadow DOM for isolation, no global CSS pollution" constraint.
+
+**Recommended extractions**
+- `services/analysis-display.ts` (pure grouping/sorting).
+- `analysis-persistence.ts` and/or add `updateRecordWithAnalysis` to `storage-service.ts`.
+- `<qd-student-entries>` Lit component (styles in `static styles`), potentially shared with the quiz overlay.
+
+### 2.4 `indexeddb.ts` (759 lines) — one class, five responsibilities
+
+- **Mixed responsibilities**: connection lifecycle + schema migration (`init` L84–241), encryption/obfuscation (`decodeStoredValue` L314–355, encrypted scan L466–519 — the adapter knows about `ENCRYPT_STORAGE`/`deriveKey`), backups (`backup` L622–666), audit log (`saveAuditEvent` L673–699), and singleton management (L715–759).
+- **Deep nesting / recursion**: `init` (157 lines) has two recursive "delete DB and retry" paths (timeout L108–136, corruption L161–191) of near-identical structure. `clearAll` (L526–612) hand-rolls a 3-way completion barrier (~86 lines).
+- **Pervasive boilerplate**: every method repeats the `new Promise(... tx.transaction ... request.onsuccess/onerror ...)` scaffold — `getStudent`, `saveStudent`, `getStudentsByRelease`, `getStudentsByReleaseEncrypted`, `backup`, `saveAuditEvent`. A `promisifyRequest<T>` + `runTransaction(store, mode, fn)` helper removes >150 lines.
+
+**Recommended extractions**
+- `promisifyRequest` / `runTransaction` helpers.
+- `connection/migration` opener module (owns `DB_VERSION`, `onupgradeneeded`, recovery).
+- `codec` layer so the adapter is unaware of encryption policy.
+- `BackupRepository` and `AuditLogRepository` sub-modules.
+- Singleton factory module.
+
+### 2.5 `bootstrap.ts` (490 lines) — god bootstrap with duplicated security logic
+
+- **CSS literal in init code**: `injectGlobalStyles` (L26–132) is a ~106-line CSS string (21% of the file) — move to `init/global-styles.ts`.
+- **Duplicated near-identical helpers**: `enhanceAllQuizTables` / `enhanceAllAnalysisTables` / `enhanceHomeBadgesIfPresent` (L236–306) collapse into one parameterized helper.
+- **Embedded + DUPLICATED business rule (security-sensitive)**: `revealQuizAnswersForInstructor` (L313–376) is almost verbatim duplicated by `event-coordinator.ts` `upgradeTablesAfterLogin`'s instructor branch (L155–201). Both unhide columns 2/3 and re-inject `question.correctAnswer` into the DOM. This answer-reveal logic must live in **one** shared enhancer function.
+- **pageId-from-URL parsing triplicated**: L314–317, L422–424 here + `event-coordinator.ts` L138–141 → extract `getPageIdFromUrl()` util.
+
+---
+
+## 3. MEDIUM-severity hot-spots
+
+### 3.1 `event-coordinator.ts` (339 lines) — event router doing business logic
+- `upgradeTablesAfterLogin` (L137–221, ~84 lines) parses URL, branches on instructor mode, performs the duplicated answer-reveal DOM manipulation (L155–200), and upgrades tables — far above the "just route events" altitude of the rest of the class.
+- The inline login-handler body (`registerLoginHandlers` L80–132) does IndexedDB load/save + cache building. Extract to storage/session layer so the coordinator only dispatches.
+
+### 3.2 `session.ts` (443 lines) — two modules glued together
+- `SessionService` class (L25–254) = session lifecycle; free functions `buildCacheFromRecord` / `buildPageCache` / `registerPageQuestions` / `updateCacheWithAnswer` (L256–420) = pure cache math already consumed independently by `storage-service.ts`. Split the cache utilities into `session-cache.ts` (drops the file under 300 lines).
+- Magic key `'qd/instructor/showAnswers'` (L125) is repeated in `bootstrap.ts` (L369) and `event-coordinator.ts` (L196) — promote to `STORAGE_KEYS`.
+
+### 3.3 `qd-migration-dialog.ts` (519 lines)
+- `validatePassword` (L319–339) duplicates `qd-login`'s SHA-256 check → shared `instructor-auth` helper.
+- `runMigration` (L344–372) computes migration `direction` from `ENCRYPT_STORAGE` — that policy belongs in a `migrationService.run(...)`.
+- ~165 lines are `static styles`; common button/input/error/spinner styles should move into the existing `shared-styles.ts`. Candidate `qd-spinner` reusable element.
+
+### 3.4 `qd-instructor.ts` (380 lines) — duplicated load routine
+- `loadStudents` logic is copy-pasted 4× (L149, L179, L221, L273): read session → `getStudentsByRelease` → set `this.students` → catch. Extract one `refreshStudents()`.
+
+### 3.5 `qd-pin-reset-dialog.ts` (389 lines)
+- `executeReset` (L243–299) embeds storage init + `resetPin` + `saveStudent` + audit-log construction (`crypto.randomUUID()` L260–267) in the dialog → `pinResetService.reset(student)`.
+- The student-search-table (L318–360) + `filteredStudents` getter (L170–178) → reusable `qd-student-table` / `qd-student-picker` (also usable by scores/export).
+
+---
+
+## 4. LOW-severity / leave-as-is
+
+- `home-badges.ts` (239): cleanest enhancer; converting badges to Lit would break progressive enhancement. Only worthwhile fix: a single `clearBadges(link)` helper (loop duplicated at L51–53, L101–105, L157–160).
+- `qd-status.ts` (354): healthy; minor — move `calculatePercentage` into `calculation-helpers.ts`.
+- `qd-pin-create.ts` (364): clean; adopt the shared `qd-modal` base instead of hand-rolled overlay; use shared `sanitizePinInput`.
+- `validation.ts` (364): well-factored pure predicates; optional `validateQuestionRow` extraction only.
+- `storage-service.ts` (283): thin coordinator; extract `createEmptyStudentRecord(session)` (duplicated at L74–84 and L91–101).
+- `contracts.ts` (411): frozen contracts — do not modify without version bump/migration.
+
+---
+
+## 5. Cross-cutting themes (fix once, benefit everywhere)
+
+1. **Persist-then-style-DOM-then-emit duplicated** in `saveAnswer` (quiz) and `saveCellData` (analysis) → shared `persistAndNotify(record, { onSavedDom, events })`.
+2. **Instructor answer-reveal logic duplicated** across `bootstrap.ts` and `event-coordinator.ts` (security-sensitive) → one shared enhancer function.
+3. **DOM config reads bypass `readDOMConfig()`** in `qd-login`, `qd-pin-reset-dialog`, `qd-migration-dialog` → route through `CONFIG_IDS`.
+4. **Instructor-password SHA-256 hashing duplicated** in `qd-login` + `qd-migration-dialog` → shared `instructor-auth`.
+5. **Magic strings** (`'qd/instructor/showAnswers'`) and **pageId-from-URL parsing** repeated 3× each → `STORAGE_KEYS` + `getPageIdFromUrl()`.
+6. **Shared styles** (`button.primary`, `.error-message`, `.button-row`, spinner, modal overlay) copy-pasted across dialogs → consolidate into `shared-styles.ts`.
+7. **Session-state checks** (`updateVisibility` reading session/instructor flags) re-implemented in `qd-login`, `qd-status`, `qd-instructor` → shared `isStudentLoggedIn()` / `isInstructor()`.
+
+---
+
+## 6. Recommended sequencing (lowest risk → highest payoff)
+
+1. **Quick, safe wins**: shared constants (`STORAGE_KEYS`), `getPageIdFromUrl()`, `clearBadges()`, `createEmptyStudentRecord()`, consolidate styles. No behavior change.
+2. **Fix the `innerHTML` XSS** in `quiz-table.ts` (security; small change).
+3. **De-duplicate the instructor answer-reveal logic** into one shared enhancer (removes a security-sensitive fork).
+4. **Extract pure logic & helpers**: `analysis-display.ts`, `promisifyRequest`/`runTransaction`, `instructor-auth`, route config through `readDOMConfig()`.
+5. **Extract services**: `AuthService` (from `qd-login`), `pinResetService`, `updateRecordWithAnalysis`.
+6. **Split large files**: decompose `indexeddb.ts`, `quiz-table.ts`, `analysis-table.ts`, `bootstrap.ts`, `session.ts` into the modules above.
+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.
diff --git a/specs/012-code-review/spec.md b/specs/012-code-review/spec.md
new file mode 100644
index 0000000..8152d4d
--- /dev/null
+++ b/specs/012-code-review/spec.md
@@ -0,0 +1,135 @@
+# Feature Specification: Refactor Architectural Hot-Spots for Maintainability
+
+**Feature Branch**: `claude/speckit-code-review-8dd0zw`
+**Created**: 2026-06-17
+**Status**: Draft
+**Input**: User description: "We have some quite large modules. I suspect we also have some quite complex blocks of logic. Do a code review, to consider `hot-spots` where: UI and business logic are too tightly intertwined; business logic is several layers deep; child components could be refactored out; UI elements could be refactored to `Lit` components; files are very large (over 400 lines). Produce a report of recommendations."
+
+## Overview
+
+The codebase has accumulated several large, multi-responsibility modules where presentation, business logic, persistence, and security-sensitive behavior are intertwined. A code review was performed against five hot-spot criteria (tight UI/logic coupling, deeply nested logic, extractable child components, UI suitable for Lit components, files over 400 lines). The findings are captured in `code-review-report.md` in this directory.
+
+This specification turns those findings into a prioritized, independently shippable refactoring effort. The goal is improved maintainability and reduced risk **without changing observable behavior** and while honoring all existing constitutional constraints (offline-first, progressive enhancement, TDD, answer security, ≤40KB bundle).
+
+The companion **`code-review-report.md` is the primary deliverable requested** ("produce a report of recommendations"); this spec defines the work the report implies so it can flow into `/speckit.plan` and `/speckit.tasks`.
+
+## Clarifications
+
+### Session 2026-06-17
+
+- Q: Should the deliverable be only a report, or a report plus the refactoring it recommends? → A: The report is the immediate deliverable; the refactoring is scoped here as prioritized, independently shippable slices so it can be planned and executed incrementally.
+- Q: Is any behavior change acceptable? → A: No. All refactoring must be behavior-preserving; one observable bug (the quiz instructor-overlay `innerHTML` XSS) is the single sanctioned behavior fix because it is a security defect.
+
+## User Scenarios & Testing *(mandatory)*
+
+### User Story 1 - Maintainer receives a prioritized hot-spot report (Priority: P1)
+
+A maintainer wants to understand where the codebase is hardest to change and why, with concrete, actionable recommendations ranked by payoff so they can plan refactoring work.
+
+**Why this priority**: This is exactly what the user asked for ("produce a report of recommendations"). It delivers value on its own — even with zero code changes, the team gains a shared, evidence-based map of technical debt.
+
+**Independent Test**: Open `code-review-report.md`; verify every file over 400 lines is listed with a severity and a concrete split recommendation, and that each of the five hot-spot criteria is addressed with file/line references.
+
+**Acceptance Scenarios**:
+
+1. **Given** the report, **When** a maintainer looks for large files, **Then** every `src/**` file over 400 lines is listed with line count, severity, and a concrete extraction plan.
+2. **Given** the report, **When** a maintainer looks for tightly coupled UI/logic, **Then** specific methods are named with line references and a recommended service/component extraction.
+3. **Given** the report, **When** a maintainer wants to start work, **Then** a recommended sequencing (lowest-risk → highest-payoff) is provided.
+
+---
+
+### User Story 2 - Eliminate duplicated and security-sensitive logic (Priority: P1)
+
+A maintainer wants the highest-risk findings resolved first: the quiz instructor-overlay XSS, the duplicated instructor answer-reveal logic, and the duplicated authentication/PIN paths.
+
+**Why this priority**: These items carry security and correctness risk (unescaped student input rendered via `innerHTML`; security-sensitive answer-reveal logic maintained in two places that can drift). Fixing them reduces real risk with small, contained changes.
+
+**Independent Test**: Student-supplied names/answers containing markup render as inert text in the instructor overlay; instructor answer-reveal behaves identically whether reached via initial bootstrap or post-login, driven by a single shared function.
+
+**Acceptance Scenarios**:
+
+1. **Given** a student answer containing `<script>` or HTML, **When** an instructor reveals answers on a quiz page, **Then** the content is displayed as literal text and not interpreted as markup.
+2. **Given** an instructor logs in, **When** answers are revealed on page load and after login, **Then** both paths invoke one shared answer-reveal function with identical results.
+3. **Given** the consolidated authentication path, **When** a student logs in normally and after a storage migration, **Then** both flows share one code path with no duplicated success logic.
+
+---
+
+### User Story 3 - Decompose oversized modules into focused units (Priority: P2)
+
+A maintainer wants the largest modules split into single-responsibility files so each is easier to read, test, and change, with no file in scope exceeding ~400 lines after the split (except frozen contracts).
+
+**Why this priority**: Size and mixed responsibility are the root causes of slow, error-prone changes, but splitting is lower-risk to defer than the security/duplication fixes in P1.
+
+**Independent Test**: After refactoring, the targeted files (`qd-login.ts`, `quiz-table.ts`, `indexeddb.ts`, `analysis-table.ts`, `bootstrap.ts`, `session.ts`) are each under ~400 lines, with extracted modules covered by unit tests, and all existing tests pass unchanged.
+
+**Acceptance Scenarios**:
+
+1. **Given** `indexeddb.ts`, **When** decomposed, **Then** connection/migration, encryption codec, backups, and audit log live in separate units and shared transaction boilerplate is replaced by a reusable helper.
+2. **Given** `qd-login.ts`, **When** authentication is extracted into a service, **Then** the component contains only presentation/state-mapping logic and the duplicated retry-after-migration path is removed.
+3. **Given** `bootstrap.ts`, **When** decomposed, **Then** the CSS literal, the table-enhancement loops, and the instructor answer-reveal rule no longer live in the bootstrap sequencer.
+
+---
+
+### User Story 4 - Extract reusable UI into Lit components (Priority: P3)
+
+A maintainer wants repeated, raw-DOM UI blocks converted into reusable, Shadow-DOM-isolated Lit components to remove inline styling and string-built markup.
+
+**Why this priority**: Improves consistency and removes a class of styling/escaping bugs, but is the least urgent and depends on the structural extractions in P2/P3 landing first.
+
+**Independent Test**: The instructor answer/entry overlays, the student-search table, and the shared spinner render via Lit components with styles in `static styles`; no hard-coded inline hex colors remain in enhancer code.
+
+**Acceptance Scenarios**:
+
+1. **Given** the instructor overlays, **When** rendered, **Then** they use Lit components (`<qd-student-answers>` / `<qd-student-entries>`) with auto-escaped bindings and no inline `style.cssText`.
+2. **Given** dialogs needing password entry, a spinner, or a student table, **When** rendered, **Then** they reuse shared components/styles rather than re-implementing them.
+
+---
+
+### Edge Cases
+
+- **Behavior drift during extraction**: a moved function changes a default or ordering. Mitigated by characterization tests written before moving logic (TDD).
+- **Bundle size regression**: adding new Lit components could grow the bundle. Each slice must verify ≤40KB min+gzip.
+- **Offline/`file://` constraint**: no extraction may introduce dynamic imports or network dependencies in the IIFE build.
+- **Frozen contracts**: `contracts.ts` must not be modified by this work.
+- **Singleton/init ordering**: extracting storage/session init must preserve the existing initialization order and the lenient singleton-by-dbName semantics relied upon by callers.
+
+## Requirements *(mandatory)*
+
+### Functional Requirements
+
+- **FR-001**: The review report MUST list every `src/**` file over 400 lines with its line count, a severity rating, and a concrete decomposition recommendation.
+- **FR-002**: The report MUST address all five requested hot-spot criteria (UI/logic coupling, deep nesting, extractable child components, Lit-candidate UI, oversized files) with specific file and line references.
+- **FR-003**: The report MUST provide a recommended execution order ranked from lowest-risk to highest-payoff.
+- **FR-004**: The instructor quiz-answer overlay MUST render student-supplied content as inert text (no `innerHTML` interpolation of student data).
+- **FR-005**: Security-sensitive instructor answer-reveal logic MUST exist in exactly one shared location used by both the initial-load and post-login paths.
+- **FR-006**: Authentication, PIN, and migration-retry logic MUST be consolidated so the normal and post-migration login flows share a single code path with no duplicated success logic.
+- **FR-007**: After decomposition, each targeted module MUST have a single primary responsibility, and no in-scope file (excluding frozen `contracts.ts`) SHOULD exceed ~400 lines.
+- **FR-008**: Duplicated cross-cutting logic (DOM config reads, instructor-password hashing, pageId-from-URL parsing, persist-then-notify, magic key strings) MUST be replaced by shared helpers/constants.
+- **FR-009**: Repeated raw-DOM UI blocks identified in the report SHOULD be reusable Lit components with styles in `static styles` and no inline hex-color styling in enhancer code.
+- **FR-010**: All refactoring MUST be behavior-preserving except the sanctioned XSS fix (FR-004); existing tests MUST pass without modification beyond relocation, and new units MUST be covered by tests.
+- **FR-011**: Every change MUST satisfy the Definition of Done (typecheck, lint, unit + integration tests, format, build) and keep the IIFE bundle ≤40KB min+gzip with no new network/dynamic-import dependencies.
+
+### Key Entities
+
+- **Hot-spot finding**: a recommendation with location (file + line range), category (one of the five criteria), severity, and a proposed extraction.
+- **Extracted unit**: a new service, helper module, or Lit component carved from an oversized module, with a single responsibility and its own tests.
+- **Cross-cutting helper**: a shared constant/function replacing logic duplicated across modules.
+
+## Success Criteria *(mandatory)*
+
+### Measurable Outcomes
+
+- **SC-001**: 100% of `src/**` files over 400 lines appear in the report with severity and a decomposition plan.
+- **SC-002**: After P1–P3 work, the six targeted oversized modules are each under ~400 lines (frozen `contracts.ts` excluded).
+- **SC-003**: Zero instances remain of student-supplied data rendered via `innerHTML` in enhancer overlays.
+- **SC-004**: The instructor answer-reveal logic and the student-login success path each exist in exactly one location (no duplicated copies).
+- **SC-005**: All existing automated tests continue to pass and newly extracted units have test coverage; the build remains green and the bundle stays ≤40KB min+gzip.
+- **SC-006**: Each refactoring slice is independently shippable — it can be merged on its own without requiring later slices to be useful.
+
+## Assumptions
+
+- The report (`code-review-report.md`) is the immediate, primary deliverable; the implied refactoring is scoped here for later planning/execution and need not all land at once.
+- "Over 400 lines" is the user's stated threshold for "very large"; `contracts.ts` (411, frozen) is explicitly out of scope for splitting.
+- Behavior preservation is required; the one exception is the quiz-overlay XSS, treated as a security defect fix rather than a behavior change.
+- Line references in the report are point-in-time and must be re-verified before editing.
+- No constitutional constraint may be relaxed by this work (offline-first, progressive enhancement, TDD, answer security, performance budget).

From fe6ab6ea99d923d3007680022a6eba14c65e1f46 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Wed, 17 Jun 2026 06:16:18 +0000
Subject: [PATCH 2/3] Fix pre-existing Prettier formatting in qd-scores-modal
 story

Unblocks the repo-wide format:check, which was failing on this file
(already broken on main) independently of this PR's documentation changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvXAFvUpR6jxvQYTofu4eG
---
 stories/components/qd-scores-modal.stories.ts | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/stories/components/qd-scores-modal.stories.ts b/stories/components/qd-scores-modal.stories.ts
index 3dad188..2c7dc63 100644
--- a/stories/components/qd-scores-modal.stories.ts
+++ b/stories/components/qd-scores-modal.stories.ts
@@ -256,10 +256,7 @@ export const Interactive: Story = {
           View Student Scores
         </button>
 
-        <qd-scores-modal
-          .students=${sampleStudents}
-          @close=${() => {}}
-        ></qd-scores-modal>
+        <qd-scores-modal .students=${sampleStudents} @close=${() => {}}></qd-scores-modal>
 
         <div
           style="margin-top: 20px; padding: 15px; background: #e3f2fd; border-radius: 4px; font-size: 14px;"

From f51e9bd3f84ae29eb4194dfecd23e760f63403d6 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Wed, 17 Jun 2026 06:34:21 +0000
Subject: [PATCH 3/3] Add implementation plan and design artifacts for hot-spot
 refactor

Phase 0/1 planning output for the code-review refactoring feature:
- plan.md: technical context, constitution check (PASS), module map
- research.md: 7 refactoring-strategy decisions (D1-D7)
- data-model.md: no persisted changes; module responsibility map
- contracts/module-boundaries.md: internal interfaces for extracted units
- quickstart.md: TDD per-slice workflow and 5-wave sequencing

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvXAFvUpR6jxvQYTofu4eG
---
 CLAUDE.md                                     |   2 +
 .../contracts/module-boundaries.md            | 129 ++++++++++++++++++
 specs/012-code-review/data-model.md           |  61 +++++++++
 specs/012-code-review/plan.md                 | 120 ++++++++++++++++
 specs/012-code-review/quickstart.md           |  79 +++++++++++
 specs/012-code-review/research.md             |  62 +++++++++
 6 files changed, 453 insertions(+)
 create mode 100644 specs/012-code-review/contracts/module-boundaries.md
 create mode 100644 specs/012-code-review/data-model.md
 create mode 100644 specs/012-code-review/plan.md
 create mode 100644 specs/012-code-review/quickstart.md
 create mode 100644 specs/012-code-review/research.md

diff --git a/CLAUDE.md b/CLAUDE.md
index 9076a65..f05cf0c 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -559,6 +559,8 @@ function getStorageKey(release: ReleaseId, serviceId: ServiceId): string {
 - N/A (CSS-only feature) (010-css-answer-hiding)
 - YAML (GitHub Actions workflows), Bash (scripts) + GitHub Actions (actions/checkout, actions/setup-node, softprops/action-gh-release) (011-release-automation)
 - N/A (CI/CD infrastructure only) (011-release-automation)
+- TypeScript 5.x / ES2020+ + Lit 3.x (Web Components), Vite 5.x (build), Vitest 2.x (unit/integration), Playwright 1.x (E2E) (012-code-review)
+- IndexedDB (primary, via `IndexedDBStorageAdapter`), sessionStorage (active session + R/A/G cache) — no schema changes in this feature (012-code-review)
 
 ## Recent Changes
 - 001-security-refactor: Added TypeScript 5.x / JavaScript ES2020+ + Lit 3.0 (Web Components), Vite 5.x (build), Vitest (testing)
diff --git a/specs/012-code-review/contracts/module-boundaries.md b/specs/012-code-review/contracts/module-boundaries.md
new file mode 100644
index 0000000..206e35d
--- /dev/null
+++ b/specs/012-code-review/contracts/module-boundaries.md
@@ -0,0 +1,129 @@
+# Phase 1 Contracts: Module Boundaries
+
+**Feature**: Refactor Architectural Hot-Spots for Maintainability
+**Date**: 2026-06-17
+
+This feature exposes no HTTP/GraphQL API and adds no public package API. The relevant "contracts" are the **internal module interfaces** of the extracted units. These are the seams the refactor must honor; they double as the unit-test surface. All types reference existing frozen types in `src/types/contracts.ts` (unchanged). Signatures below are indicative TypeScript and may be adjusted during implementation as long as the responsibility and behavior are preserved.
+
+## Auth
+
+```ts
+// services/auth/auth-service.ts
+type LoginResult =
+  | { kind: 'success'; session: SessionData }
+  | { kind: 'new-student-created'; session: SessionData }
+  | { kind: 'lockout'; untilMs: number }
+  | { kind: 'bad-pin'; remaining: number }
+  | { kind: 'needs-migration'; error: string };
+
+interface StudentLoginInput { serviceId: ServiceId; name: string; pin?: string; }
+
+class AuthService {
+  loginStudent(input: StudentLoginInput): Promise<LoginResult>;
+  retryAfterMigration(input: StudentLoginInput): Promise<LoginResult>;
+}
+```
+**Contract**: All authentication, lockout, PIN, and migration-detection logic lives here; the component performs no storage/crypto/rate-limit calls. `loginStudent` and `retryAfterMigration` share one internal success path (no duplication).
+
+```ts
+// services/auth/instructor-auth.ts
+function hashPassword(plain: string): Promise<string>;            // SHA-256, 12-char truncation
+function verifyInstructorPassword(plain: string): Promise<boolean>; // compares against configured hash
+```
+**Contract**: The single source of instructor-password hashing; `qd-login` and `qd-migration-dialog` both consume it (no duplicated crypto).
+
+## PIN reset
+
+```ts
+// services/pin-reset-service.ts
+interface PinResetResult { ok: boolean; error?: string; }
+function resetStudentPin(student: StudentRecord): Promise<PinResetResult>; // resets PIN + writes audit event
+```
+**Contract**: Storage init, `resetPin`, `saveStudent`, and audit-event construction are encapsulated; the dialog only renders the result.
+
+## Storage
+
+```ts
+// services/storage/idb-helpers.ts
+function promisifyRequest<T>(req: IDBRequest<T>, op: string): Promise<T>;
+function runTransaction<T>(db: IDBDatabase, store: string, mode: IDBTransactionMode,
+                          fn: (s: IDBObjectStore) => IDBRequest<T>): Promise<T>;
+
+// services/storage/idb-codec.ts  (encryption-aware; adapter is NOT)
+function encodeForStore(value: unknown): Promise<unknown>;
+function decodeStoredValue(raw: unknown): Promise<unknown>; // throws StorageFormatError on mismatch
+
+// services/storage/idb-connection.ts
+function openDatabase(dbName: string): Promise<IDBDatabase>;  // owns DB_VERSION, onupgradeneeded, recovery
+
+// services/storage/backup-repository.ts
+function createBackup(db: IDBDatabase, payload: BackupRecord): Promise<void>;
+
+// services/storage/audit-log-repository.ts
+function saveAuditEvent(db: IDBDatabase, event: AuditEvent): Promise<void>;
+```
+**Contract**: `IndexedDBStorageAdapter` becomes a thin coordinator delegating to these units. No method re-implements transaction scaffolding. Encryption policy is confined to `idb-codec`. Singleton-by-dbName semantics are preserved.
+
+```ts
+// services/storage-service.ts  (additions)
+function updateRecordWithAnalysis(record: StudentRecord, /* cell args */): StudentRecord; // mirror of updateRecordWithAnswer
+function createEmptyStudentRecord(session: SessionData): StudentRecord;                    // dedupe try/catch literals
+```
+
+## Session
+
+```ts
+// services/session-cache.ts  (pure functions moved from session.ts)
+function buildCacheFromRecord(record: StudentRecord): SessionCache;
+function buildPageCache(/* ... */): PageCache;
+function registerPageQuestions(/* ... */): void;
+function updateCacheWithAnswer(cache: SessionCache, /* ... */): SessionCache;
+```
+**Contract**: `SessionService` retains only session lifecycle; cache math is import-only and DOM-free (already consumed independently by `storage-service.ts`).
+
+## Enhancers
+
+```ts
+// enhancers/instructor-answer-reveal.ts  (single shared, security-sensitive)
+function revealInstructorAnswers(table: HTMLTableElement, metadata: QuizTableMetadata): void;
+function hideInstructorAnswers(table: HTMLTableElement): void;
+
+// enhancers/quiz-answer-persistence.ts
+function saveAnswer(/* ... */): Promise<void>;          // storage + cache + events
+function applyValidationStyling(cell: HTMLElement, success: boolean): void; // DOM-only
+
+// enhancers/quiz-input-factory.ts
+function createQuestionInput(spec: QuestionInputSpec): HTMLElement;
+
+// enhancers/quiz-table-columns.ts
+function hideAnswerColumn(table: HTMLTableElement): void;
+function showAnswerColumn(table: HTMLTableElement): void;
+function hideDetailColumn(table: HTMLTableElement): void;
+function removeColgroup(table: HTMLTableElement): void;
+
+// services/analysis-display.ts  (pure)
+function groupEntriesByCell(/* ... */): GroupedEntries;
+function sortByTimestamp(/* ... */): StudentEntry[];
+
+// utils/page-id.ts
+function getPageIdFromUrl(url?: string): PageId;
+```
+**Contract**: `revealInstructorAnswers` is the only place correct answers are re-injected into the DOM; both `bootstrap.ts` and `event-coordinator.ts` call it. Persistence and DOM-styling are separated so each is independently testable.
+
+## Reusable Lit components (attributes/events)
+
+```text
+<qd-student-answers .answers=${StudentAnswerDisplay[]}>      // auto-escaped; replaces innerHTML overlay
+<qd-student-entries .entries=${StudentEntryDisplay[]}>       // styles in static styles; no inline cssText
+<qd-student-table .students=${StudentRecord[]} @select=${e}> // searchable; emits per-row action
+<qd-spinner>                                                  // shared loading indicator
+<qd-lockout-banner .untilMs=${number} @expired=${e}>        // optional; owns its countdown
+```
+**Contract**: All bindings auto-escape; all styles live in `static styles` (Shadow DOM, no global CSS). Shared visual styles come from `shared-styles.ts`. Adding these must keep the IIFE bundle ≤40KB min+gzip (per-slice `size-check`).
+
+## Invariants across all contracts
+
+1. No new network calls or dynamic imports (offline-first / IIFE-safe).
+2. No changes to `src/types/contracts.ts` or persisted data shapes.
+3. Every extracted unit is covered by tests; existing tests pass unchanged except relocation.
+4. Behavior is preserved everywhere except the quiz-overlay XSS fix (FR-004).
diff --git a/specs/012-code-review/data-model.md b/specs/012-code-review/data-model.md
new file mode 100644
index 0000000..213f4fc
--- /dev/null
+++ b/specs/012-code-review/data-model.md
@@ -0,0 +1,61 @@
+# Phase 1 Data Model: Refactor Architectural Hot-Spots
+
+**Feature**: Refactor Architectural Hot-Spots for Maintainability
+**Date**: 2026-06-17
+
+## Persisted data: NO CHANGES
+
+This is an internal, behavior-preserving refactor. **No persisted entities are added, removed, or modified.** The frozen contracts in `src/types/contracts.ts` (`AnswerRecord`, `StudentRecord`, `PageData`, `SessionData`, `SessionCache`, identity types) remain untouched. IndexedDB stores (`students`, `backups`, audit log), the composite key scheme `qd/{release}/u{serviceId}`, and the 30-minute session timeout are unchanged.
+
+The "data model" for this feature is therefore the **module/responsibility map** — the units being created and the contracts they expose — not new domain data.
+
+## Refactor entities (engineering artifacts)
+
+### Hot-spot finding
+A reviewed location flagged for refactoring.
+- **Fields**: file path, line range, category (one of: tight-coupling, deep-nesting, extractable-component, lit-candidate, oversized-file), severity (HIGH/MEDIUM/LOW), recommended extraction.
+- **Source**: enumerated in `code-review-report.md`.
+- **Lifecycle**: open → addressed (by a slice) → verified (tests green, file under threshold).
+
+### Extracted unit
+A new module/component carved from an oversized file.
+- **Fields**: name, kind (service | helper | repository | Lit component | pure-logic module), source module(s), public interface, owning test file.
+- **Invariant**: single primary responsibility; covered by unit tests; introduces no new runtime/network dependency.
+
+### Cross-cutting helper
+A shared constant/function replacing duplicated logic.
+- **Fields**: name, replaces (list of duplicate sites), location.
+- **Examples**: `getPageIdFromUrl()`, `instructor-auth.hashPassword()`, `STORAGE_KEYS.INSTRUCTOR_SHOW_ANSWERS`, `idb-helpers.promisifyRequest()`, `clearBadges()`, `createEmptyStudentRecord()`, `persistAndNotify()`.
+
+## Module responsibility map (target state)
+
+| New/changed unit | Kind | Carved from | Single responsibility |
+|------------------|------|-------------|-----------------------|
+| `services/auth/auth-service.ts` | service | `qd-login.ts` | Student login + retry-after-migration; returns result union |
+| `services/auth/instructor-auth.ts` | helper | `qd-login.ts`, `qd-migration-dialog.ts` | SHA-256 instructor-password hashing/verification |
+| `services/pin-reset-service.ts` | service | `qd-pin-reset-dialog.ts` | Reset PIN + write audit event |
+| `services/analysis-display.ts` | pure-logic | `analysis-table.ts` | `groupEntriesByCell`, `sortByTimestamp` |
+| `services/session-cache.ts` | pure-logic | `session.ts` | `buildCacheFromRecord` & related cache math |
+| `services/storage/idb-helpers.ts` | helper | `indexeddb.ts` | `promisifyRequest`, `runTransaction` |
+| `services/storage/idb-connection.ts` | module | `indexeddb.ts` | Open/upgrade/recover the DB |
+| `services/storage/idb-codec.ts` | module | `indexeddb.ts` | Encryption-aware encode/decode |
+| `services/storage/backup-repository.ts` | repository | `indexeddb.ts` | Backups |
+| `services/storage/audit-log-repository.ts` | repository | `indexeddb.ts` | Audit log |
+| `enhancers/quiz-answer-persistence.ts` | module | `quiz-table.ts` | Save quiz answer + validation styling |
+| `enhancers/quiz-table-columns.ts` | helper | `quiz-table.ts` | Column show/hide |
+| `enhancers/quiz-input-factory.ts` | helper | `quiz-table.ts` | Build MCQ/numeric inputs |
+| `enhancers/quiz-instructor-overlay.ts` | module | `quiz-table.ts` | Show/hide student answers |
+| `enhancers/analysis-persistence.ts` | module | `analysis-table.ts` | Save analysis cell |
+| `enhancers/analysis-instructor-overlay.ts` | module | `analysis-table.ts` | Show/hide student entries |
+| `enhancers/instructor-answer-reveal.ts` | module | `bootstrap.ts`, `event-coordinator.ts` | Single shared answer-reveal routine |
+| `init/global-styles.ts` | asset | `bootstrap.ts` | Global CSS literal |
+| `utils/page-id.ts` | helper | `bootstrap.ts`, `event-coordinator.ts` | `getPageIdFromUrl()` |
+| `components/qd-student-answers.ts` | Lit component | `quiz-table.ts` | Render student answers (escaped) |
+| `components/qd-student-entries.ts` | Lit component | `analysis-table.ts` | Render student entries (encapsulated styles) |
+| `components/qd-student-table.ts` | Lit component | `qd-pin-reset-dialog.ts` | Searchable student table |
+| `components/qd-spinner.ts` | Lit component | `qd-migration-dialog.ts` | Loading spinner |
+| `components/qd-lockout-banner.ts` | Lit component (optional) | `qd-login.ts` | Lockout countdown |
+
+## State transitions
+
+No runtime state machines change. The only "state" here is the refactor workflow per finding: **open → addressed → verified**, with `verified` requiring (a) characterization/unit tests green, (b) the source file under the ~400-line threshold where applicable, and (c) Definition of Done passing.
diff --git a/specs/012-code-review/plan.md b/specs/012-code-review/plan.md
new file mode 100644
index 0000000..6ffda75
--- /dev/null
+++ b/specs/012-code-review/plan.md
@@ -0,0 +1,120 @@
+# Implementation Plan: Refactor Architectural Hot-Spots for Maintainability
+
+**Branch**: `claude/speckit-code-review-8dd0zw` (spec dir `012-code-review`) | **Date**: 2026-06-17 | **Spec**: [spec.md](./spec.md)
+**Input**: Feature specification from `/specs/012-code-review/spec.md`
+
+## Summary
+
+Decompose the codebase's largest, most entangled modules into single-responsibility units and remove duplicated/security-sensitive logic, **without changing observable behavior** (the one sanctioned exception being the quiz instructor-overlay `innerHTML` XSS fix). The companion [code-review-report.md](./code-review-report.md) is the primary deliverable and the source of all findings; this plan sequences the implied refactoring into independently shippable slices.
+
+Technical approach: behavior-preserving extraction guided by characterization tests (write/confirm tests against current behavior → extract → keep green). Work proceeds in risk order: shared constants/helpers → security/duplication fixes → service extraction → file decomposition → reusable Lit components. Each slice independently satisfies the Definition of Done and keeps the IIFE bundle ≤40KB min+gzip.
+
+## Technical Context
+
+**Language/Version**: TypeScript 5.x / ES2020+
+**Primary Dependencies**: Lit 3.x (Web Components), Vite 5.x (build), Vitest 2.x (unit/integration), Playwright 1.x (E2E)
+**Storage**: IndexedDB (primary, via `IndexedDBStorageAdapter`), sessionStorage (active session + R/A/G cache) — no schema changes in this feature
+**Testing**: Vitest (unit + integration), Playwright (E2E against Storybook), Chromatic (visual regression)
+**Target Platform**: Chrome/Edge ≥96, Firefox ≥102, must run from `file://` URLs
+**Project Type**: Single project (browser library, progressive enhancement of DITA HTML)
+**Performance Goals**: <200ms save, <2s page load (50 questions); no regression introduced by refactor
+**Constraints**: IIFE bundle ≤40KB min+gzip; no network/dynamic imports; Shadow DOM isolation (no global CSS); behavior-preserving (except the FR-004 XSS fix); `src/types/contracts.ts` is FROZEN and out of scope
+**Scale/Scope**: ~14.4k LOC across `src/`; 8 modules in scope (6 oversized + `event-coordinator.ts` + cross-cutting helpers). No new persisted entities.
+
+No NEEDS CLARIFICATION items remain — scope, constraints, and target modules are fully determined by the spec and report.
+
+## Constitution Check
+
+*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
+
+- [x] **Offline-First**: Pure internal refactor; no network dependencies added. Verified by retained `file://` E2E.
+- [x] **Progressive Enhancement**: Enhancers continue to upgrade existing DITA HTML; `home-badges` deliberately left as class-toggle enhancement (not converted to Lit) to preserve graceful degradation.
+- [x] **Test-Driven Development**: Characterization tests written/confirmed before each extraction; new units get unit tests; Red-Green-Refactor enforced (FR-010).
+- [x] **Phase-Gated Delivery**: This plan defines slices with explicit exit criteria; each slice is independently shippable (SC-006).
+- [x] **Performance Constraints**: Each slice verifies ≤40KB min+gzip and Shadow-DOM isolation; new Lit components add styles via `static styles` (no global CSS). `size-check` is a per-slice gate.
+- [x] **Data Isolation**: Composite key scheme and 30-min session timeout unchanged; no key/namespace changes. `contracts.ts` untouched.
+- [x] **Zero Configuration**: No new config, script tags, or attributes; routing config reads through the existing `readDOMConfig()` actually *reduces* divergence.
+
+**Result: PASS** — no violations; Complexity Tracking not required.
+
+### Re-check after Phase 1 design
+
+- [x] Module boundaries (contracts/) introduce no new runtime dependencies, no new persisted data, and no public-API/`contracts.ts` changes. Reusable Lit components keep styles encapsulated. **Still PASS.**
+
+## Project Structure
+
+### Documentation (this feature)
+
+```text
+specs/012-code-review/
+├── spec.md                  # Feature spec
+├── code-review-report.md    # PRIMARY deliverable: prioritized findings
+├── plan.md                  # This file
+├── research.md              # Phase 0 output: refactoring strategy decisions
+├── data-model.md            # Phase 1 output: entities (unchanged) + new module map
+├── quickstart.md            # Phase 1 output: per-slice DoD workflow
+├── contracts/
+│   └── module-boundaries.md # Phase 1 output: target module interfaces
+└── checklists/
+    └── requirements.md      # Spec quality checklist
+```
+
+### Source Code (repository root)
+
+Current (in-scope) layout and the modules this refactor introduces:
+
+```text
+src/
+├── components/
+│   ├── qd-login.ts                     # SHRINK → thin view; extract AuthService
+│   ├── qd-migration-dialog.ts          # use instructor-auth + migrationService
+│   ├── qd-pin-reset-dialog.ts          # extract pinResetService; use qd-student-table
+│   ├── qd-instructor/qd-instructor.ts  # add refreshStudents()
+│   ├── qd-student-table.ts             # NEW reusable component (pin-reset/scores/export)
+│   ├── qd-spinner.ts                   # NEW reusable component
+│   └── qd-lockout-banner.ts            # NEW (optional) reusable component
+├── enhancers/
+│   ├── quiz-table.ts                   # SPLIT (see below); fix innerHTML XSS
+│   ├── quiz-table-columns.ts           # NEW (column show/hide)
+│   ├── quiz-input-factory.ts           # NEW (createQuestionInput)
+│   ├── quiz-answer-persistence.ts      # NEW (handleAnswerInput/saveAnswer)
+│   ├── quiz-instructor-overlay.ts      # NEW (show/hide student answers)
+│   ├── analysis-table.ts               # SPLIT (see below)
+│   ├── analysis-persistence.ts         # NEW (saveCellData)
+│   ├── analysis-instructor-overlay.ts  # NEW
+│   ├── instructor-answer-reveal.ts     # NEW shared (dedupes bootstrap/event-coordinator)
+│   └── home-badges.ts                  # minor: clearBadges() helper only
+├── services/
+│   ├── auth/
+│   │   ├── auth-service.ts             # NEW (loginStudent/retryAfterMigration)
+│   │   └── instructor-auth.ts          # NEW (shared SHA-256 hashing)
+│   ├── pin-reset-service.ts            # NEW
+│   ├── analysis-display.ts             # NEW (pure groupEntriesByCell/sortByTimestamp)
+│   ├── session.ts                      # SPLIT → session-cache.ts
+│   ├── session-cache.ts               # NEW (pure cache math)
+│   ├── storage-service.ts              # add updateRecordWithAnalysis, createEmptyStudentRecord
+│   └── storage/
+│       ├── indexeddb.ts                # SPLIT (see below)
+│       ├── idb-helpers.ts             # NEW (promisifyRequest/runTransaction)
+│       ├── idb-connection.ts          # NEW (init/migration/recovery)
+│       ├── idb-codec.ts               # NEW (encode/decode, encryption-aware)
+│       ├── backup-repository.ts       # NEW
+│       └── audit-log-repository.ts    # NEW
+├── init/
+│   ├── bootstrap.ts                    # SHRINK → thin sequencer
+│   ├── global-styles.ts                # NEW (CSS literal moved out)
+│   └── event-coordinator.ts            # SHRINK → event routing only
+├── config/dom-config-reader.ts         # existing; become the single config-read path
+└── utils/
+    └── page-id.ts                      # NEW getPageIdFromUrl()
+
+tests/
+├── unit/        # new unit tests for each extracted module
+└── integration/ # characterization tests for enhancers/storage behavior
+```
+
+**Structure Decision**: Single-project browser library. The refactor adds new files alongside existing directories (`services/auth/`, `services/storage/`, `enhancers/`) following patterns already present in the repo (e.g. `services/answer-display.ts`, the `qd-instructor/` component family). No directory reorganization of existing healthy files.
+
+## Complexity Tracking
+
+No constitution violations — section intentionally empty.
diff --git a/specs/012-code-review/quickstart.md b/specs/012-code-review/quickstart.md
new file mode 100644
index 0000000..d257e2f
--- /dev/null
+++ b/specs/012-code-review/quickstart.md
@@ -0,0 +1,79 @@
+# Quickstart: Executing the Hot-Spot Refactor
+
+**Feature**: Refactor Architectural Hot-Spots for Maintainability
+**Date**: 2026-06-17
+
+This guide describes how to execute the refactor slice-by-slice. Read [code-review-report.md](./code-review-report.md) for the findings and [plan.md](./plan.md) for the module map.
+
+## Golden rule
+
+**Behavior-preserving, test-gated, one slice at a time.** The only sanctioned behavior change in the entire feature is the quiz instructor-overlay XSS fix (FR-004).
+
+## Per-slice workflow (TDD)
+
+For every extraction:
+
+1. **Characterize**: write/confirm tests that capture the *current* behavior of the target code (return values, emitted `qd:*` events, IndexedDB/sessionStorage writes, DOM mutations). Run them — they pass against the old code.
+2. **Extract**: move the logic into the new module/component per `contracts/module-boundaries.md`. Re-point callers.
+3. **Green**: run tests — behavior unchanged. Re-verify the new unit has its own focused unit tests.
+4. **Definition of Done** (all must pass with zero errors):
+   ```bash
+   npm run typecheck
+   npm run lint
+   npm run test:unit
+   npm run test:integration   # if enhancers/storage touched
+   npm run format:check
+   npm run build
+   npm run size-check         # ≤40KB min+gzip — REQUIRED gate for any Lit-component slice
+   ```
+5. **Commit** the slice with a descriptive message. Each slice is independently shippable.
+
+## Recommended slice order (waves)
+
+### Wave 1 — Safe consolidation (no behavior change)
+- `STORAGE_KEYS.INSTRUCTOR_SHOW_ANSWERS` constant (replaces `'qd/instructor/showAnswers'` magic string ×3).
+- `utils/page-id.ts` `getPageIdFromUrl()` (replaces 3 inline parses).
+- `home-badges` `clearBadges(link)` helper (replaces 3 duplicated loops).
+- `storage-service` `createEmptyStudentRecord(session)` (dedupe try/catch literals).
+- Consolidate shared styles (`button.primary`, `.error-message`, `.button-row`, spinner, modal overlay) into `shared-styles.ts`.
+
+### Wave 2 — Security & duplication (highest risk removed)
+- **Fix the `innerHTML` XSS** in the quiz instructor overlay (FR-004) — render via `textContent`/component.
+- Extract `enhancers/instructor-answer-reveal.ts` and call it from both `bootstrap.ts` and `event-coordinator.ts` (FR-005).
+- Extract `services/auth/instructor-auth.ts`; route `qd-login` + `qd-migration-dialog` through it.
+
+### Wave 3 — Pure logic & services
+- `services/analysis-display.ts` (pure grouping/sorting).
+- `services/session-cache.ts` (split from `session.ts`).
+- `services/storage/idb-helpers.ts` (`promisifyRequest`/`runTransaction`).
+- `services/auth/auth-service.ts` (collapses the duplicated login/retry paths — FR-006).
+- `services/pin-reset-service.ts`; `storage-service.updateRecordWithAnalysis`.
+- Route all DOM config reads through `config/dom-config-reader.ts` (`readDOMConfig`).
+
+### Wave 4 — Large-file decomposition (target ≤400 lines each)
+- `indexeddb.ts` → `idb-connection`, `idb-codec`, `backup-repository`, `audit-log-repository`.
+- `quiz-table.ts` → `quiz-table-columns`, `quiz-input-factory`, `quiz-answer-persistence`, `quiz-instructor-overlay`.
+- `analysis-table.ts` → `analysis-persistence`, `analysis-instructor-overlay`.
+- `bootstrap.ts` → `init/global-styles.ts` + collapse `enhanceAll*` helpers; move business rules out.
+- `event-coordinator.ts` → event routing only (login IO + table-upgrade moved out).
+- `qd-login.ts` → thin view over `AuthService`.
+
+### Wave 5 — Reusable Lit components
+- `<qd-student-answers>`, `<qd-student-entries>` (back the overlays; remove inline `style.cssText`).
+- `<qd-student-table>` (pin-reset/scores/export), `<qd-spinner>`, optional `<qd-lockout-banner>`.
+- Adopt the shared `qd-modal` base in `qd-pin-create.ts`.
+
+## Verifying success criteria
+
+| Criterion | How to verify |
+|-----------|---------------|
+| SC-002 (files <400 lines) | `find src -name '*.ts' | xargs wc -l | awk '$1>400'` returns only `contracts.ts` |
+| SC-003 (no student data via `innerHTML`) | grep enhancer overlays for `innerHTML`; none with student fields |
+| SC-004 (single answer-reveal / login path) | one definition of `revealInstructorAnswers`; no `retryLoginAfterMigration` duplicate |
+| 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 |
+
+## Out of scope (do not touch)
+- `src/types/contracts.ts` (frozen).
+- `home-badges` → Lit conversion (breaks progressive enhancement).
+- Any persisted data shape, key scheme, or session-timeout change.
diff --git a/specs/012-code-review/research.md b/specs/012-code-review/research.md
new file mode 100644
index 0000000..7c85b1d
--- /dev/null
+++ b/specs/012-code-review/research.md
@@ -0,0 +1,62 @@
+# Phase 0 Research: Refactoring Strategy
+
+**Feature**: Refactor Architectural Hot-Spots for Maintainability
+**Date**: 2026-06-17
+
+There are no unknown technologies to research — the stack (TypeScript 5.x, Lit 3.x, Vite, Vitest, Playwright, IndexedDB) is established. The open questions for this feature are *strategy* decisions about how to refactor safely. Each is resolved below.
+
+## D1: How to guarantee behavior preservation during extraction
+
+- **Decision**: Use characterization tests. Before moving any logic, write/confirm tests that capture current observable behavior of the target function (inputs → outputs/DOM/events/storage). Extract the logic, re-point callers, keep tests green. Only then refactor internals.
+- **Rationale**: The codebase mandates TDD (Constitution III) and most target modules lack unit isolation today. Characterization tests turn "behavior-preserving" (FR-010) into a verifiable gate rather than an aspiration.
+- **Alternatives considered**: Pure manual verification (rejected — not repeatable, violates TDD); snapshot-only tests (rejected — too brittle for DOM-heavy enhancers, prefer explicit assertions on events/storage).
+
+## D2: Sequencing of slices (risk vs. payoff)
+
+- **Decision**: Five waves — (1) shared constants/helpers (no behavior change), (2) security + duplication fixes (XSS, answer-reveal dedup, login-path dedup), (3) pure-logic & service extraction, (4) large-file decomposition, (5) reusable Lit components.
+- **Rationale**: Front-loads zero-risk consolidation that *reduces* duplication the later waves would otherwise have to move twice; isolates the one sanctioned behavior change (XSS) early where it's easy to review; defers UI componentization (highest churn, bundle-size sensitive) until structure is clean.
+- **Alternatives considered**: File-by-file top-down (rejected — would re-do shared-helper extraction repeatedly); big-bang rewrite of `qd-login`/`indexeddb` (rejected — not independently shippable, high regression risk).
+
+## D3: Boundary for the `AuthService` extraction from `qd-login.ts`
+
+- **Decision**: `AuthService.loginStudent(input)` returns a discriminated union (`{kind:'success'} | {kind:'lockout', untilMs} | {kind:'bad-pin', remaining} | {kind:'needs-migration', error} | {kind:'new-student-created'}`). The component maps results → state only. `retryAfterMigration(...)` shares the same internal success path.
+- **Rationale**: Removes the ~100-line duplicated `retryLoginAfterMigration`, flattens 4–5-level nesting, and makes auth unit-testable without a DOM. Discriminated unions keep the component's render logic exhaustive and type-safe.
+- **Alternatives considered**: Throwing typed errors for each outcome (rejected — control-flow via exceptions is harder to test exhaustively); boolean flags (rejected — recreates the nesting we're removing).
+
+## D4: Encryption awareness in the IndexedDB split
+
+- **Decision**: Introduce an `idb-codec` layer (`read`/`write`) that owns `ENCRYPT_STORAGE`, `deriveKey`, encode/decode, and format-mismatch detection. The adapter and repositories call `codec.read/write` and remain unaware of encryption policy. A shared `promisifyRequest`/`runTransaction` helper replaces the repeated `new Promise(...)` scaffold.
+- **Rationale**: Encryption is currently leaking into the persistence layer (`indexeddb.ts`); isolating it shrinks the adapter, removes ~150 lines of boilerplate, and keeps the obfuscation feature (009) cohesive. Connection/migration moves to `idb-connection`; backups and audit log become small repositories.
+- **Alternatives considered**: Leaving codec inline (rejected — keeps the file >700 lines and the worst nesting); a full repository/ORM abstraction (rejected — over-engineering for two stores; violates "simpler alternative" preference).
+
+## D5: Which UI to convert to Lit (and which NOT to)
+
+- **Decision**: Convert the instructor overlays (`<qd-student-answers>`, `<qd-student-entries>`), the student-search table (`<qd-student-table>`), and small shared elements (`<qd-spinner>`, optional `<qd-lockout-banner>`). **Do NOT** convert `home-badges` — it stays a CSS-class enhancement on existing DITA nav links.
+- **Rationale**: The overlays currently build markup via `innerHTML`/`style.cssText`, which is the source of the XSS (quiz) and the Shadow-DOM-violating inline colors (analysis); Lit auto-escapes and encapsulates styles. Badges, by contrast, are progressive enhancement of existing markup — converting them to Lit would break graceful degradation (Constitution II).
+- **Alternatives considered**: Convert everything to Lit (rejected — breaks PE for badges, grows bundle); keep raw DOM but manually escape (rejected — leaves duplicated rendering and inline styles, doesn't address Constitution V isolation).
+
+## D6: Where the shared instructor answer-reveal logic lives
+
+- **Decision**: One exported function in `enhancers/instructor-answer-reveal.ts` (e.g. `revealInstructorAnswers(table, metadata)`), called by both `bootstrap.ts` (initial load) and `event-coordinator.ts` (post-login).
+- **Rationale**: This security-sensitive logic (re-injecting `correctAnswer` into the DOM and unhiding columns) is currently copy-pasted in two places that can drift (FR-005). Co-locating it with the quiz enhancer keeps answer handling in the layer responsible for Constitution VIII.
+- **Alternatives considered**: Keep two copies with a shared test (rejected — still two maintenance points); push into `qd-instructor` component (rejected — it operates on enhancer-owned table DOM, wrong layer).
+
+## D7: Bundle-size safety for new Lit components
+
+- **Decision**: Treat `npm run size-check` (≤40KB min+gzip) as a per-slice exit gate, measured after each Lit component is added. Share styles via `shared-styles.ts` to avoid duplicating CSS across new components.
+- **Rationale**: Lit components are the only wave that can grow the bundle (Constitution V). Measuring per-slice catches regressions at the responsible change rather than at the end.
+- **Alternatives considered**: Measure only at the end (rejected — hard to attribute a regression); skip new components if near budget (deferred — revisit only if a measured slice breaches budget).
+
+## Summary of resolved decisions
+
+| ID | Decision |
+|----|----------|
+| D1 | Characterization tests gate every extraction |
+| D2 | Five risk-ordered waves; constants → security → services → decomposition → Lit |
+| D3 | `AuthService` returns a discriminated union; component is a thin view |
+| D4 | `idb-codec` + `promisifyRequest`/`runTransaction`; adapter unaware of encryption |
+| D5 | Lit-ify overlays/table/spinner; keep `home-badges` as PE class toggles |
+| D6 | Single shared `revealInstructorAnswers` used by bootstrap + event-coordinator |
+| D7 | `size-check` is a per-slice gate; share styles to protect the 40KB budget |
+
+All NEEDS CLARIFICATION resolved. Ready for Phase 1.