Refactor architectural hot-spots for maintainability (012 — complete)#94
Merged
Conversation
Consolidate the duplicated ~100-line student-login flow from qd-login's handleStudentLogin and retryLoginAfterMigration into a shared AuthService (loginStudent / retryAfterMigration) returning a discriminated LoginResult. The component now only handles validation, session creation, events, and UI. Route DOM config reads (title selector, db name) through dom-config-reader helpers instead of inline document.getElementById calls in qd-login and qd-pin-reset-dialog. Adds characterization tests for the login outcomes (T015). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
Move the pure cache-building helpers (buildCacheFromRecord, buildPageCache, registerPageQuestions, updateCacheWithAnswer) out of session.ts into session-cache.ts so SessionService retains only session lifecycle. Re-point storage-service and quiz-table imports; relocate the cache unit tests to tests/unit/session-cache.test.ts. session.ts is now under 300 lines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
Split the 759-line indexeddb.ts into a thin coordinator (308 lines) plus: - idb-connection.ts: schema, DB_VERSION, store names, open/upgrade/recovery - idb-helpers.ts: promisifyRequest / runTransaction scaffolding - idb-codec.ts: encryption-aware encode/decode + format-mismatch checks - backup-repository.ts / audit-log-repository.ts: non-student stores The adapter no longer references ENCRYPT_STORAGE/deriveKey or re-implements transaction plumbing; clearAll uses a single atomic transaction without the hand-rolled completion barrier. Adds characterization (indexeddb-adapter) and idb-helpers unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
Decompose the 824-line quiz-table.ts (now 399) into: - quiz-table-columns.ts: answer/detail column show/hide + removeColgroup - quiz-input-factory.ts: createQuestionInput - quiz-answer-persistence.ts: handleAnswerInput/saveAnswer/applyValidationStyling - quiz-instructor-overlay.ts: show/hide student answers quiz-table.ts retains only lifecycle/orchestration. Re-point instructor- answer-reveal and the overlay integration test to the new overlay module. Existing quiz-table integration suite is the characterization net (T023). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
Decompose the 637-line analysis-table.ts (now 301) into: - services/analysis-display.ts: pure groupEntriesByCell/sortByTimestamp + CellEntry - enhancers/analysis-persistence.ts: handleCellEdit/saveCellData - enhancers/analysis-instructor-overlay.ts: createStudentEntriesDisplay, show/hide entries, getCurrentPageId Add StorageService.updateRecordWithAnalysis (mirrors updateRecordWithAnswer) so the persistence path no longer hand-builds analysis records. analysis-table.ts retains only lifecycle/orchestration. Re-point test imports; add a focused analysis-display unit test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
Contributor
🚀 PR Preview Deployment✅ Your changes are being built and deployed to a preview environment. 📋 Live PR Preview Links: ℹ️ About PR Previews:
📦 Production Demo (main branch): 🧪 Testing Locally: npm install
npm run build
# Open demo/quiz-index.html in your browser (use file:// or http-server)This comment will be automatically updated when you push new commits. |
… (US3) Move the ~95-line global CSS literal out of bootstrap.ts into init/global-styles.ts and collapse the duplicated enhanceAllQuizTables/enhanceAllAnalysisTables helpers into one parameterized enhanceAllTables. bootstrap.ts is now 321 lines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
…partial T043) Move the ~130-line static styles block out of qd-login.ts (813 -> 683) into a dedicated styles module, keeping the component focused on behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
…S3 T043) Extract the instructor flow into <qd-instructor-login> (button + password modal + verification) and the lockout countdown into <qd-lockout-banner> (owns its own timer, emits qd:lockout-expired). qd-login (398 lines) now only renders the student form and maps AuthService results to UI state. Add a shared readRelease() config helper and component tests for both new components. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
Move the ~155-line static styles block out of qd-migration-dialog.ts (495 -> 343) into a dedicated styles module. With this, no src file except the frozen contracts.ts exceeds 400 lines (SC-002). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
Move the login-handler cache-rebuild IO into StorageService.refreshCacheOnLogin and the post-login table upgrade into enhancers/table-upgrade.ts. The coordinator (216 lines) now only routes events to those collaborators and the shared reveal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
Cover global-style injection, non-interactive table enhancement, and the existing-session upgrade-to-interactive path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
…S4 T046) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
…ent (US4 T048) Replace the raw-DOM builder that set inline style.cssText hex colors (Constitution V) with a Shadow-DOM-isolated <qd-student-entries> component: styles in static styles, auto-escaped bindings. Update the analysis-display and analysis-table tests to query the component shadow root. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
…(US4 T044/T047) Replace the raw-DOM answer overlay with a Shadow-DOM <qd-student-answers> component (auto-escaped bindings, encapsulated styles) and remove the now- redundant global student-answer CSS. Add component tests asserting escaped rendering for both <qd-student-answers> and <qd-student-entries>. Update the quiz-instructor-overlay test to query the component shadow root. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
…() (US4 T050) Collapse the four near-identical getStudentsByRelease load/try-catch blocks in qd-instructor into a single refreshStudents() method used by every panel/handler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
…alog (US4 T045/T049) Replace the dialog's inline searchable table with the reusable <qd-student-table> (searchable, emits per-row select) and route the reset through pin-reset-service so the dialog only renders results. Removes the duplicated search/filter state and table styles. Adds qd-student-table component tests (search filter, select event, auto-escaped rendering). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
- Add utils/session-state.ts (hasActiveSession/isInstructor/isStudentLoggedIn) and dedupe the three updateVisibility implementations in qd-login, qd-status, qd-instructor. - Add enhancers/persist-and-notify.ts (save record -> rebuild cache -> DOM -> events) and use it in quiz-answer-persistence and analysis-persistence. - Use the shared calculatePercentage helper in qd-status and sanitizePinInput in qd-pin-create. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
…054-T056) All 56 tasks complete. SC-002 holds (only frozen contracts.ts >400 lines), SC-003 holds (no innerHTML for student data in enhancer overlays; overlays backed by auto-escaped Lit components), full DoD green, bundle 37.6KB gzip (<40KB). Append a resolution table to code-review-report.md with final line counts for every former hot-spot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfnhLV87VA1735fQ9Evsj3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes feature 012-code-review: behavior-preserving, test-gated decomposition of the codebase's largest, most entangled modules into single-responsibility units, plus de-duplication of security-sensitive logic. All 56 tasks in
specs/012-code-review/tasks.mdare done.The only sanctioned behavior change is the quiz instructor-overlay
innerHTML→textContentXSS fix (FR-004). Every commit passes the full Definition of Done (typecheck,lint,test:unit,test:integration,format:check,build,size-check).Hot-spots resolved (final line counts)
qd-login.tsquiz-table.tsstorage/indexeddb.tsanalysis-table.tsqd-migration-dialog.tsinit/bootstrap.tsservices/session.tsinit/event-coordinator.tsWhat's in here
AuthService(consolidates the duplicated ~100-line login flow),instructor-auth, config reads viadom-config-reader, sharedinstructor-answer-reveal, quiz-overlay XSS fix.idb-connection/idb-helpers/idb-codec/backup-repository/audit-log-repository;session-cache; quiz-table → columns/input-factory/answer-persistence/instructor-overlay; analysis-table →analysis-display/analysis-persistence/analysis-instructor-overlay; bootstrap →global-styles+ parameterized enhancement; event-coordinator reduced to a pure router;qd-login→ presentational view.<qd-spinner>,<qd-student-answers>,<qd-student-entries>,<qd-student-table>;pin-reset-service.session-state(dedupesupdateVisibility×3),persist-and-notify(dedupes the save→cache→events tail), sharedcalculatePercentage/sanitizePinInput.Verification (Success Criteria)
src/types/contracts.ts(411) exceeds 400 lines. ✅innerHTML; overlays are backed by auto-escaped Lit components. ✅🤖 Generated with Claude Code