Skip to content

feat: imported-course mode for needs-analysis, assessment-design, course-builder - #9

Merged
savvides merged 2 commits into
mainfrom
feat/imported-course-mode
May 2, 2026
Merged

feat: imported-course mode for needs-analysis, assessment-design, course-builder#9
savvides merged 2 commits into
mainfrom
feat/imported-course-mode

Conversation

@savvides

@savvides savvides commented May 2, 2026

Copy link
Copy Markdown
Owner

Summary

Three skills (needs-analysis, assessment-design, course-builder) assumed net-new course design and broke down for courses imported from a cartridge. The TMC-430 test report flagged this as Cluster #C — three issues with the same root cause. This PR adds an early branch on `import_metadata.source` to each affected skill.

Skill Without this PR (TMC-430 case) With this PR
needs-analysis Asks "is training the right intervention?" — meaningless for a credit-bearing course already in the registrar's catalog. Skips the gate, records `justified=true, confidence=10, rationale="design audit only"`, runs a design-fit check instead.
assessment-design Designs new rubrics from scratch even though the cartridge has 6 existing ones. New Mode 3 — Audit Existing: reads existing rubrics, classifies criteria on Bloom's, flags alignment gaps. Doesn't propose new assessments unless asked.
course-builder Would regenerate syllabus, modules, rubrics that already exist. Half-measure at line 211 acknowledged `import_metadata` but didn't act on it. New gap-fill mode: generates ONLY the artifacts upstream skills flagged as missing. Skips Steps 2–7 entirely.

All three announce the chosen mode at the start of the conversation, so the user can override the auto-detection if it's wrong.

Mode-detection contract

Both conditions must be true to enter the audit-existing/gap-fill branch:

  1. `import_metadata.source` is one of `cartridge`, `scorm`, `canvas-api` (whitelisted formats only).
  2. The skill-relevant section is non-empty:
    • needs-analysis: `import_metadata.items_imported.modules > 0`
    • assessment-design: `assessments.items` non-empty OR `course_content.assessments` non-empty
    • course-builder: `course_content.modules` non-empty

Default is design-new mode — the conservative choice. Borderline states (e.g., `import_metadata.source` set but zero modules imported) ask the user via `AskUserQuestion` rather than guessing.

Schema additions (additive, no version bump)

New optional fields in `templates/manifest-schema.md`:

  • `needs_analysis.mode` — `"design-new"` or `"audit-existing"`
  • `assessments.mode` — `"Mode 1"`, `"Mode 2"`, or `"Mode 3"`
  • `assessments.audit_notes[]` — Mode 3 outputs (`{target_id, action, description, reason}`)
  • `course_content.mode` — `"build-new"` or `"gap-fill"`
  • `course_content.recommended_generation_targets[]` — gap-fill outputs (`{description, source, status, output_path}`)

Existing manifests stay readable; downstream skills that don't read the new fields ignore them.

Other changes

  • assessment-design adopts `bin/idstack-manifest-merge` for its manifest writes (landed in v2.1.0.0). Replaces the inline-write pattern's misleading "include the COMPLETE schema structure" instruction with two scoped merge calls (one per top-level section the skill writes).

Test plan

  • Smoke tests: 153/153 passing (was 150). 3 new assertions: each mode-aware skill template references `import_metadata.source` (drift guard against accidental removal).
  • Generated freshness: `bin/idstack-gen-skills --dry-run` — 0 stale.
  • Manifest schema review: new optional fields are additive; `bin/idstack-migrate` LATEST_VERSION stays at 1.4 (no migration needed).
  • Out-of-band on TMC-430 (after merge): the regression target. Expected:
    • `needs-analysis` reports "Mode: audit-existing (cartridge import from Canvas)" instead of "Mode: design-new"; skips the training-justification gate.
    • `assessment-design` reports "Mode 3: 6 existing rubrics audited; 2 alignment gaps found" instead of designing new rubrics.
    • `course-builder` reports "Mode: gap-fill; 4 generation targets queued" (matching the targets the v2.1.0.0 audit identified) instead of attempting to generate a syllabus.

Risks (covered)

  • R1 — Mode-detection edge cases: explicit decision tree, both whitelist + non-empty check; ambiguous states ask the user.
  • R2 — Design-new branch silently regresses: audit-existing branch is strictly additive before existing workflow; no edits to existing Step bodies.
  • R3 — Item shape inconsistency between modes: both modes produce items in the canonical shape (same `id, type, title, weight, ilos_measured, rubric_present, alignment_status` fields).
  • R4 — Test coverage: smoke-test grep is a weak signal but acceptable; behavioral verification is the TMC-430 re-run.
  • R5 — User confusion when manual `import_metadata` is partial: mode is announced as the first sentence, user can override.

Out of scope (deferred — see TMC-430 test report)

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces 'Imported-course mode' for the needs-analysis, assessment-design, and course-builder skills, enabling specialized workflows for imported content such as auditing existing assessments and gap-fill generation. The manifest schema is updated with mode-tracking fields and audit note arrays, and the assessment-design skill is updated to use the idstack-manifest-merge tool for atomic writes. Feedback identifies misleading section descriptions in the assessment-design skill and corrects field name inconsistencies in the course-builder skill to ensure alignment with the canonical schema.

Comment thread skills/assessment-design/SKILL.md Outdated
Comment on lines +630 to +632
and atomically updates the top-level `updated` timestamp. **This skill writes two
sections** — `assessments` and `learning_objectives.alignment_matrix.ilo_to_assessment` —
so call the merge tool twice (once per top-level section).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The description of the sections written by this skill is misleading. It identifies learning_objectives.alignment_matrix.ilo_to_assessment as a "section", but the idstack-manifest-merge tool operates on top-level sections (like learning_objectives). Furthermore, in Mode 3, the skill also writes to learning_objectives.alignment_matrix.gaps. It is clearer and more accurate to just name the top-level sections.

Suggested change
and atomically updates the top-level `updated` timestamp. **This skill writes two
sections**`assessments` and `learning_objectives.alignment_matrix.ilo_to_assessment`
so call the merge tool twice (once per top-level section).
and atomically updates the top-level `updated` timestamp. **This skill writes two
sections**`assessments` and `learning_objectives`
so call the merge tool twice (once per top-level section).

Comment thread skills/assessment-design/SKILL.md.tmpl Outdated
Comment on lines +452 to +454
and atomically updates the top-level `updated` timestamp. **This skill writes two
sections** — `assessments` and `learning_objectives.alignment_matrix.ilo_to_assessment` —
so call the merge tool twice (once per top-level section).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The description of the sections written by this skill is misleading. It identifies learning_objectives.alignment_matrix.ilo_to_assessment as a "section", but the idstack-manifest-merge tool operates on top-level sections (like learning_objectives). Furthermore, in Mode 3, the skill also writes to learning_objectives.alignment_matrix.gaps. It is clearer and more accurate to just name the top-level sections.

and atomically updates the top-level `updated` timestamp. **This skill writes two
sections** — `assessments` and `learning_objectives` —
so call the merge tool twice (once per top-level section).

Comment thread skills/course-builder/SKILL.md Outdated
Comment on lines +380 to +384
4. **Record outputs.** Update `course_content.generated_files` (additive) and `course_builder_notes.recommended_generation_targets` (the list, with status `generated | deferred | declined`).

When done, write the manifest via `bin/idstack-manifest-merge` (see Write Manifest below) and skip directly to the final summary.

Save the chosen mode under `course_builder_notes.mode` (`"build-new"` or `"gap-fill"`).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The instructions refer to course_builder_notes for the mode and recommended_generation_targets fields, but the manifest schema (and the generated_files field in the same sentence) uses course_content. This inconsistency will lead to the LLM writing data to a non-existent section.

Suggested change
4. **Record outputs.** Update `course_content.generated_files` (additive) and `course_builder_notes.recommended_generation_targets` (the list, with status `generated | deferred | declined`).
When done, write the manifest via `bin/idstack-manifest-merge` (see Write Manifest below) and skip directly to the final summary.
Save the chosen mode under `course_builder_notes.mode` (`"build-new"` or `"gap-fill"`).
4. **Record outputs.** Update `course_content.generated_files` (additive) and `course_content.recommended_generation_targets` (the list, with status `generated | deferred | declined`).
When done, write the manifest via `bin/idstack-manifest-merge` (see Write Manifest below) and skip directly to the final summary.
Save the chosen mode under `course_content.mode` (`"build-new"` or `"gap-fill"`).

Comment thread skills/course-builder/SKILL.md.tmpl Outdated
Comment on lines +202 to +206
4. **Record outputs.** Update `course_content.generated_files` (additive) and `course_builder_notes.recommended_generation_targets` (the list, with status `generated | deferred | declined`).

When done, write the manifest via `bin/idstack-manifest-merge` (see Write Manifest below) and skip directly to the final summary.

Save the chosen mode under `course_builder_notes.mode` (`"build-new"` or `"gap-fill"`).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The instructions refer to course_builder_notes for the mode and recommended_generation_targets fields, but the manifest schema (and the generated_files field in the same sentence) uses course_content. This inconsistency will lead to the LLM writing data to a non-existent section.

4. **Record outputs.** Update `course_content.generated_files` (additive) and `course_content.recommended_generation_targets` (the list, with status `generated | deferred | declined`).

When done, write the manifest via `bin/idstack-manifest-merge` (see Write Manifest below) and skip directly to the final summary.

Save the chosen mode under `course_content.mode` (`"build-new"` or `"gap-fill"`).

savvides pushed a commit that referenced this pull request May 2, 2026
Two corrections from Gemini Code Assist's review of this PR:

- assessment-design: the manifest-write text said "writes two sections —
  assessments and learning_objectives.alignment_matrix.ilo_to_assessment" but
  the merge tool operates on top-level sections only, and Mode 3 also writes
  to learning_objectives.alignment_matrix.gaps. Clarified to "writes two
  top-level sections — assessments and learning_objectives" with a note about
  reading-then-merging the full learning_objectives section before passing
  it to the merge tool.

- course-builder: gap-fill mode instructions referenced
  course_builder_notes.recommended_generation_targets and
  course_builder_notes.mode, but the canonical schema (and the
  course_content.generated_files reference in the same sentence) puts these
  under course_content. Real bug — Claude following the skill would write to
  a section that isn't whitelisted by bin/idstack-manifest-merge. Renamed
  both to course_content.

Smoke test 153/153 unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
philippos savvides and others added 2 commits May 2, 2026 09:51
…rse-builder

Three skills assumed net-new course design and broke down for imported courses
(TMC-430 test report issues #7, #13, #14). Each now branches early on
import_metadata.source:

- needs-analysis: skips the "is training the right intervention?" decision
  gate for credit-bearing imports (the registrar can't be told to remove the
  course); records justification automatically and runs a design-fit check.

- assessment-design: adds Mode 3 (Audit Existing Assessments) alongside the
  existing Mode 1/Mode 2 split. Reads existing rubrics, classifies on Bloom's,
  compares to ILOs, surfaces alignment gaps. Does NOT propose new assessments
  unless asked. Mode 3 takes precedence over Mode 1.

- course-builder: adds gap-fill mode triggered when import_metadata.source is
  set and course_content.modules is non-empty. Generates ONLY artifacts
  upstream skills flagged as missing, instead of regenerating syllabus,
  modules, and rubrics that already exist in the cartridge.

All three skills announce the chosen mode at the start of the conversation,
so the user can override if auto-detection is wrong.

Canonical schema gains optional needs_analysis.mode, assessments.mode,
course_content.mode (record the mode), assessments.audit_notes[] (Mode 3
output), and course_content.recommended_generation_targets[] (gap-fill
output). Additive — no schema version bump.

assessment-design also switches its manifest write to bin/idstack-manifest-merge
(landed in v2.1.0.0), eliminating the misleading "include the COMPLETE schema
structure" instruction.

Smoke test grows from 150 to 153 assertions: each mode-aware skill template
must reference import_metadata.source (drift guard).

Resolves test-report issues #7, #13, #14.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two corrections from Gemini Code Assist's review of this PR:

- assessment-design: the manifest-write text said "writes two sections —
  assessments and learning_objectives.alignment_matrix.ilo_to_assessment" but
  the merge tool operates on top-level sections only, and Mode 3 also writes
  to learning_objectives.alignment_matrix.gaps. Clarified to "writes two
  top-level sections — assessments and learning_objectives" with a note about
  reading-then-merging the full learning_objectives section before passing
  it to the merge tool.

- course-builder: gap-fill mode instructions referenced
  course_builder_notes.recommended_generation_targets and
  course_builder_notes.mode, but the canonical schema (and the
  course_content.generated_files reference in the same sentence) puts these
  under course_content. Real bug — Claude following the skill would write to
  a section that isn't whitelisted by bin/idstack-manifest-merge. Renamed
  both to course_content.

Smoke test 153/153 unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@savvides
savvides force-pushed the feat/imported-course-mode branch from 3e57254 to 116efe5 Compare May 2, 2026 16:52
@savvides
savvides merged commit 0824065 into main May 2, 2026
@savvides
savvides deleted the feat/imported-course-mode branch May 2, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant