feat(themis): Module coherence with smart titles and overview-first generation

[JasonWarrenUK]

Overview

This PR implements a two-part enhancement to Themis that eliminates content repetition across modules and provides flexible AI-guided title generation.

Problem Solved

Previously, generated modules would repeat content from earlier modules because:

• No rich context about what learners had already covered
• Title requirements forced literal strings like "Module 1" that polluted generation context
• No intermediate review step before committing to expensive full generation

Solution

1. Smart Title System

• Three input modes: undefined (AI decides), prompt (AI guided), or literal (exact value)
• Type-safe implementation using discriminated union types
• Backward-compatible migration for existing data

2. Overview-First Generation

• Lightweight module overviews (~30s) before full modules (~90s)
• Structured overview: learning objectives, prerequisites, key concepts
• Cumulative knowledge context passed between generations
• Explicit "DO NOT REPEAT" instructions based on previous modules

Key Benefits

✅ Reduced repetition: Each module knows what previous modules covered
✅ Faster iteration: Review 10 overviews in ~5min vs 10 full modules in ~20min
✅ Better planning: See entire course structure before committing
✅ Cost savings: Overviews are cheaper to regenerate
✅ Flexible titling: Let AI decide, guide it, or specify exact titles

Implementation Details

Type System

• TitleInput type (undefined/prompt/literal)
• ModuleOverview interface (objectives, prerequisites, concepts)
• LearnerKnowledgeContext interface (accumulated knowledge)
• New status: 'overview-ready'

UI Components

• TitleInputField.svelte - Reusable input component with three modes
• Updated ArcStructurePlanner and ModuleWithinArcPlanner with new inputs
• ModuleCard - Two-step workflow UI (Generate Overview → Generate Full Module)
• ModuleGenerationList - Overview and full generation orchestration

API Endpoints

• POST /api/themis/module/overview - Fast overview generation
• Enhanced POST /api/themis/module - Full generation with knowledge context

Utilities

• knowledgeContextBuilder.ts - Aggregates preceding module data
• migrations.ts - Auto-migration for legacy data (backward compatible)
• metisPromptFactory.ts - Overview prompt builder with context awareness

Files Changed

Created (4 files)

• src/lib/components/themis/TitleInputField.svelte
• src/routes/api/themis/module/overview/+server.ts
• src/lib/utils/themis/knowledgeContextBuilder.ts
• src/lib/utils/themis/migrations.ts

Modified (9 files)

• src/lib/types/themis.ts - Type system updates
• src/lib/schemas/apiValidator.ts - New status enum value
• src/lib/stores/themisStores.ts - Overview update logic
• src/lib/components/themis/ArcStructurePlanner.svelte
• src/lib/components/themis/ModuleWithinArcPlanner.svelte
• src/lib/components/themis/ModuleGenerationList.svelte
• src/lib/components/themis/ModuleCard.svelte
• src/lib/factories/prompts/metisPromptFactory.ts
• docs/dev/wip/module-coherence-continuation.md

Testing

Manual testing checklist completed:

• ✅ Legacy data migration (backward compatibility verified)
• ✅ Title input UI (all three modes functional)
• ✅ Overview generation (fast, structured output)
• ✅ Knowledge context accumulation (no repetition)
• ✅ Full module generation with context
• ✅ Two-step workflow end-to-end

User Workflow

Two-Step Generation (Recommended)

1. Plan arcs and module slots with flexible title inputs
2. Generate all overviews (~30s each, review structure)
3. Generate full modules (~90s each, with accumulated context)

Quick Generation (Alternative)

1. Plan module slots
2. "Skip to Full Module" (generates overview + full module in sequence)

Documentation

Complete implementation guide available in docs/dev/wip/module-coherence-continuation.md

Includes:

• Architecture & design decisions
• Complete feature reference
• Usage guide with examples
• Testing checklist
• Future enhancement roadmap

Breaking Changes

None. All changes are backward compatible:

• Existing data auto-migrates on load
• Missing titleInput converts title to literal mode
• Missing overview is gracefully handled (optional field)
• Can roll back without data loss

Commits

• fcea57c - feat(themis): add TitleInput type and module overview structure
• 5c11a58 - feat(themis): add TitleInputField component and integrate with planners
• 76afdba - feat(themis): add module overview generation API and prompt builder
• 9ddf1e2 - feat(themis): add knowledge context builder utility
• 52fc085 - feat(themis): add overview generation UI and two-step workflow

Related

• Implements Roadmap Priority Structured Input Form #2: Module Coherence & Context Awareness
• Addresses task 1.1.2.2: Flexible title/theme inputs
• Milestone 2.1: Module-to-module coherence

---

Ready for review and merge. All functionality tested and documented.

[claude]

Summary

This PR implements a well-architected enhancement to Themis that introduces flexible AI-guided title generation and overview-first module generation. The implementation demonstrates excellent type safety through discriminated unions, comprehensive backward compatibility via migration utilities, and thoughtful separation of concerns. The knowledge context builder effectively prevents content repetition across modules. Minor areas for improvement include better error handling consistency, extracting magic numbers to configuration, and adding automated tests.

🎯 Code Quality & Best Practices

Excellent Patterns

Type Safety with Discriminated Unions
The TitleInput type (src/lib/types/themis.ts:8-11) is a textbook example of proper TypeScript usage:

export type TitleInput =
  | { type: 'undefined' }
  | { type: 'prompt'; value: string }
  | { type: 'literal'; value: string };

This makes invalid states unrepresentable and provides excellent autocomplete. Well done!

Idempotent Migration Pattern
The migration utilities (src/lib/utils/themis/migrations.ts) are properly idempotent, allowing safe repeated calls. The backward compatibility approach is solid and follows best practices.

Reusable Component Design
TitleInputField.svelte is well-designed with clear props, proper event dispatching, and responsive styling. The reactive statements correctly handle prop changes.

Knowledge Context Builder Architecture
The separation of concerns in knowledgeContextBuilder.ts is excellent - pure functions, clear responsibilities, and well-documented:

• buildKnowledgeContext() - aggregation
• getPrecedingModules() - ordering logic
• hasCompleteOverview() - validation
• getKnowledgeCoverageStats() - metrics

Minor Improvements Needed

1. Component Safety Check (src/lib/components/themis/TitleInputField.svelte:10-11)

const safeValue = value || { type: 'undefined' as const };

This safety check is good, but the prop should be properly typed as required or have a proper default. Consider:

export let value: TitleInput = { type: 'undefined' };

2. Reactive Statement Side Effects (TitleInputField.svelte:24-32)
The reactive $: block that calls onChange() will trigger on every reactive dependency change, potentially causing unnecessary parent updates. Consider using a more targeted approach:

function handleTypeChange() {
  if (inputType === 'undefined') {
    onChange({ type: 'undefined' });
  } else {
    onChange({ type: inputType, value: inputValue });
  }
}

Then bind to on:change={handleTypeChange} instead of relying on reactive execution.

3. Magic Numbers Should Be Configurable
Multiple hardcoded validation thresholds appear throughout:

• src/routes/api/themis/module/overview/+server.ts:92 - timeout: 60000
• src/routes/api/themis/module/overview/+server.ts:76 - maxRetries = 3
• src/routes/api/themis/module/overview/+server.ts:164-166 - 3-5 items validation

Consider extracting to a shared configuration file:

// src/lib/config/generation.ts
export const GENERATION_CONFIG = {
  overview: {
    timeoutMs: 60000,
    maxRetries: 3,
    validation: {
      minObjectives: 3,
      maxObjectives: 5,
      minPrerequisites: 1,
      minKeyConcepts: 3,
      maxKeyConcepts: 5
    }
  }
};

4. Type Assertion Should Be Avoided
src/routes/api/themis/module/overview/+server.ts:86:

body.moduleSlot as ModuleSlot

The Zod validation should already ensure this is a valid ModuleSlot. Consider defining the Zod schema to match the TypeScript type exactly or using Zod's inferred types.

🐛 Potential Issues

Critical

1. JSON Parsing Without Error Context (overview/+server.ts:108)

const overview = JSON.parse(jsonMatch[0]);

If the AI returns malformed JSON, the error message won't include the actual response text for debugging. Consider:

try {
  const overview = JSON.parse(jsonMatch[0]);
} catch (parseError) {
  console.error('Failed to parse JSON response:', jsonMatch[0]);
  throw new Error(`JSON parse failed: ${parseError.message}`);
}

2. Inconsistent Validation Error Handling
The overview API endpoint (src/routes/api/themis/module/overview/+server.ts:111-115) passes validation errors to retry attempts, but the full module generation endpoint may not. Ensure consistency across both endpoints.

Moderate

3. Race Condition Risk in Module Generation
If a user clicks "Generate Overview" multiple times rapidly, there's no mechanism preventing concurrent requests for the same module. Consider adding request deduplication or disabling buttons during generation in ModuleGenerationList.svelte.

4. Missing Null Checks in Knowledge Context Builder
src/lib/utils/themis/knowledgeContextBuilder.ts:67:

.flatMap(m => m.overview!.keyConceptsIntroduced)

The non-null assertion operator ! assumes overview exists, but this is only filtered on line 63. While logically safe, consider being more defensive:

.flatMap(m => m.overview?.keyConceptsIntroduced || [])

5. Migration Applies to Every Store Load
src/lib/stores/themisStores.ts:58:

data = migrateCourseData(data);

While idempotent migrations are safe, running them on every localStorage load adds overhead. Consider:

• Adding a version field to detect if migration is needed
• Or migrating once and persisting the result with a flag

Minor

6. Array Deduplication Could Be More Efficient
Multiple uses of filter((item, index, self) => self.indexOf(item) === index) in knowledgeContextBuilder.ts:68, 73, 78. For large arrays, this is O(n²). Consider using Set for O(n) deduplication:

const coveredTopics = [...new Set(
  modulesWithOverviews.flatMap(m => m.overview!.keyConceptsIntroduced)
)];

7. Potential XSS in Module Card Display
The ModuleCard component binds user-controlled strings directly to the DOM. While Svelte escapes by default, ensure any HTML rendering uses {@html} carefully if added in the future.

⚡ Performance Considerations

Good Practices

1. Efficient Overview Generation
The 60-second timeout and 4096 max tokens for overview generation (overview/+server.ts:92) is appropriately tuned for the lighter workload.

2. Proper Filtering Before Iteration
The knowledge context builder correctly filters modules with overviews before iterating:

const modulesWithOverviews = precedingModules.filter(m => m.overview);

Potential Optimizations

1. Derived Store Efficiency
If moduleStatusCounts is a derived store that recomputes on every store update, consider memoization if the computation becomes expensive with many modules.

2. LocalStorage Size Concerns
Storing complete module XML in localStorage (via persistedStore) could hit browser limits (~5-10MB) for large courses. Consider:

• Compressing XML data
• Moving to IndexedDB for larger storage
• Adding storage quota monitoring

3. Unnecessary JSON Parsing
The JSON extraction regex pattern (overview/+server.ts:103):

const jsonMatch = responseText.match(/\{[\s\S]*\}/);

Will match from the first { to the last }, which could include unrelated JSON if the AI response contains multiple objects. Consider a more precise pattern or instructing the AI to wrap the response in specific delimiters.

4. Prompt Building Could Be Cached
If the same course context is used for multiple module generations, consider caching the base prompt segments to avoid rebuilding them each time.

5. Component Re-renders
The TitleInputField component's reactive block (lines 24-32) triggers on any dependency change. If used in a large list, this could cause many unnecessary parent updates. Consider using Svelte's event dispatching instead of reactive onChange calls.

🔒 Security Concerns

Low Risk

1. API Key Exposure in Logs
While the code doesn't directly log the API key, ensure that any error logging doesn't inadvertently expose it through request headers or client configurations.

2. Input Validation
The Zod schemas properly validate inputs, which is excellent. However, ensure string length limits exist to prevent DoS via extremely large inputs:

title: z.string().max(500),
description: z.string().max(5000),

3. SSRF Risk in Research Feature
If the enableResearch feature fetches external URLs based on module content, ensure proper URL validation and allowlisting of domains (as mentioned in CLAUDE.md's researchDomains.ts).

4. localStorage XSS Mitigation
Storing user input in localStorage and re-rendering it could be an XSS vector if any future code uses {@html}. Current implementation is safe, but document this for future developers.

Best Practices Applied

✅ Environment variable validation (overview/+server.ts:55-60)
✅ Proper error message sanitization
✅ Zod schema validation at API boundaries
✅ No direct SQL/command injection vectors

✅ Test Coverage

Current State

The PR description mentions "Manual testing checklist completed" with specific items verified. However, no automated tests are included in the changed files.

Critical Test Gaps

1. TitleInput Type System
Missing unit tests for:

• createTitleInput() with all three modes
• getDisplayTitle() with various inputs
• migrateCourseData() idempotency

2. Knowledge Context Builder
Should have comprehensive tests for:

describe('buildKnowledgeContext', () => {
  it('returns empty context for first module');
  it('aggregates data from multiple preceding modules');
  it('deduplicates topics and skills');
  it('handles modules without overviews gracefully');
});

describe('getPrecedingModules', () => {
  it('returns empty array for first module');
  it('includes modules from previous arcs');
  it('orders modules correctly');
});

3. API Endpoint
Missing integration tests for:

• Valid overview generation request
• Validation failure retry logic
• Malformed JSON response handling
• Timeout behavior
• Zod schema validation failures

4. Component Tests
TitleInputField.svelte should have:

• Rendering tests for all three modes
• Event dispatch verification
• Props reactivity tests
• Accessibility checks (labels, ARIA attributes)

Recommendations

Given the complexity and importance of this feature:

1. Prioritize tests for core logic:

   • knowledgeContextBuilder.ts (pure functions, easy to test)
   • migrations.ts (critical for backward compatibility)

2. Add E2E tests for the two-step workflow:

   • Create course → Plan modules → Generate overviews → Generate full modules
   • Verify no content repetition across modules

3. Consider property-based testing for migration utilities to ensure they handle all legacy data shapes correctly.

4. Add visual regression tests for the ModuleCard component showing overview data.

Testing the Knowledge Context Feature

To manually verify the anti-repetition feature works:

1. Generate 3 module overviews sequentially
2. Check that Module 2's prompt includes Module 1's concepts in "DO NOT REPEAT"
3. Check that Module 3's prompt includes Modules 1-2's concepts
4. Verify the generated objectives don't overlap

This could be automated with snapshot tests of the generated prompts.

---

Overall Assessment

Strengths:

• Excellent type safety and architecture
• Thoughtful backward compatibility
• Clear separation of concerns
• Well-documented code and comprehensive PR description

Key Recommendations:

1. Add automated tests, especially for knowledge context builder
2. Extract magic numbers to configuration
3. Improve error handling with better context
4. Consider adding request deduplication for UI interactions

Approval Status: ✅ Recommended for merge after addressing critical issues (#1 JSON parsing error context) and adding basic unit tests for core utilities.

The feature is well-implemented and the manual testing is thorough, but automated tests are essential for long-term maintenance.