Skip to content

feat(adaptive-learning): implement adaptive learning test#5113

Draft
jabbadizzleCode wants to merge 7 commits into
v3from
adaptive-learning
Draft

feat(adaptive-learning): implement adaptive learning test#5113
jabbadizzleCode wants to merge 7 commits into
v3from
adaptive-learning

Conversation

@jabbadizzleCode

@jabbadizzleCode jabbadizzleCode commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

ClickUp Links

This was generated by AI during triage.

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b6704e4b-ed4c-43e9-ae4d-c4f985912b6d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
packages/adaptive-learning/src/index.ts Adds the core adaptive-learning math and selection utilities.
packages/shared-components/src/adaptive/utils.ts Adds shared display helpers for adaptive levels and theta formatting.
packages/graphql/package.json Adds the adaptive-learning workspace package dependency.
packages/prisma/src/prisma/schema/adaptive.prisma Adds the adaptive assessment schema models.

Reviews (6): Last reviewed commit: "Merge branch 'v3' of github.com:uzh-bf/k..." | Re-trigger Greptile

Comment on lines +390 to +441
const assessment = await ctx.prisma.$transaction(async (prisma) => {
if (input.id) {
const existing = await prisma.adaptiveAssessment.findUnique({
where: { id: input.id },
})

if (!existing || existing.courseId !== input.courseId) {
throw new Error('Adaptive assessment not found.')
}

await prisma.adaptiveAssessmentResponse.deleteMany({
where: { attempt: { assessmentId: input.id } },
})
await prisma.adaptiveAssessmentAttempt.deleteMany({
where: { assessmentId: input.id },
})
await prisma.adaptiveAssessmentResultMessage.deleteMany({
where: { assessmentId: input.id },
})
await prisma.adaptiveAssessmentElement.deleteMany({
where: { assessmentId: input.id },
})
await prisma.adaptiveAssessmentSubCompetence.deleteMany({
where: { assessmentId: input.id },
})
await prisma.adaptiveAssessmentCompetence.deleteMany({
where: { assessmentId: input.id },
})
await prisma.adaptiveAssessmentLevel.deleteMany({
where: { assessmentId: input.id },
})

return prisma.adaptiveAssessment.update({
where: { id: input.id },
data: assessmentData(input, course.ownerId),
})
}

return prisma.adaptiveAssessment.create({
data: {
...assessmentData(input, course.ownerId),
courseId: input.courseId,
},
})
})

await createAdaptiveAssessmentConfig(assessment.id, input, ctx)

return ctx.prisma.adaptiveAssessment.findUnique({
where: { id: assessment.id },
include: assessmentInclude,
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Config creation runs outside the outer transaction

The first $transaction deletes all levels, competences, sub-competences, elements, and result messages for an existing assessment. createAdaptiveAssessmentConfig (which recreates them) is called after that transaction completes. If the config creation throws—invalid element mapping, DB constraint, or any transient error—the assessment is left with its config fully deleted but not recreated. At that point the assessment has no levels, no question pool, and no result messages, and there is no automatic recovery path. The delete-then-recreate should be wrapped in a single atomic $transaction so both steps either succeed together or roll back together.

Fix in Codex Fix in Claude Code

Comment on lines +400 to +406
await prisma.adaptiveAssessmentResponse.deleteMany({
where: { attempt: { assessmentId: input.id } },
})
await prisma.adaptiveAssessmentAttempt.deleteMany({
where: { assessmentId: input.id },
})
await prisma.adaptiveAssessmentResultMessage.deleteMany({

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 All student attempts and responses deleted unconditionally on every edit

upsertAdaptiveAssessment wipes every AdaptiveAssessmentAttempt and AdaptiveAssessmentResponse for an assessment every time it is saved, regardless of publication status. If a lecturer edits any field on a published assessment while students have already taken it (or are currently taking it), all historical and in-progress attempt data is silently destroyed. There is no status check (DRAFT vs PUBLISHED) here. At minimum, the deletion of attempts and responses should be skipped—or blocked entirely with an error—unless the assessment is still in DRAFT status.

Fix in Codex Fix in Claude Code

Comment on lines +86 to +88
return ordered.map((level, index) => {
const start = min + (span * index) / ordered.length
const end = min + (span * (index + 1)) / ordered.length

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Frontend band boundaries don't match backend level-assignment boundaries

mapLevelsToBands divides the theta range into N equal slices (width = span/N). mapLevelsToTheta in packages/adaptive-learning/src/index.ts places level thetas at N-1-spaced intervals and uses midpoints as boundaries (width ≈ span/(N-1)). For a typical 4-level setup with range [-3, 3] the discrepancy is visible: the frontend shows the A2/B1 boundary at -1.5 while the backend assigns B1 only for θ ≥ -2. A user at θ = 1.8 would see themselves visually inside the "C1 band" but receive the B2 label.

Suggested change
return ordered.map((level, index) => {
const start = min + (span * index) / ordered.length
const end = min + (span * (index + 1)) / ordered.length
return ordered.map((level, index) => {
const denominator = Math.max(ordered.length - 1, 1)
const theta = min + (span * index) / denominator
const prevTheta = index === 0 ? min : min + (span * (index - 1)) / denominator
const nextTheta =
index === ordered.length - 1
? max
: min + (span * (index + 1)) / denominator
const start = index === 0 ? min : (theta + prevTheta) / 2
const end = index === ordered.length - 1 ? max : (theta + nextTheta) / 2

Fix in Codex Fix in Claude Code

Comment on lines +759 to +827
await ctx.prisma.$transaction([
ctx.prisma.adaptiveAssessmentResponse.create({
data: {
attemptId,
adaptiveElementId,
elementId: adaptiveElement.elementId,
order: attempt.responses.length,
response: response as DB.Prisma.InputJsonValue,
correct: correctness,
thetaBefore: attempt.currentTheta,
thetaAfter: nextState.theta,
standardErrorAfter: nextState.standardError,
elapsedSeconds,
},
}),
ctx.prisma.adaptiveAssessmentElement.update({
where: { id: adaptiveElementId },
data: { exposure: { increment: 1 } },
}),
])

const updatedAttempt = await ctx.prisma.adaptiveAssessmentAttempt.findUnique({
where: { id: attemptId },
include: attemptInclude,
})
if (!updatedAttempt) throw new Error('Adaptive attempt not found.')

const shouldFinalize =
allCompetencesStopped(updatedAttempt) ||
!selectNextAdaptiveElement(updatedAttempt)

if (shouldFinalize) {
const level = mapThetaToLevel(
nextState.theta,
updatedAttempt.assessment.levels,
{
min: updatedAttempt.assessment.thetaMin,
max: updatedAttempt.assessment.thetaMax,
}
)

await ctx.prisma.adaptiveAssessmentAttempt.update({
where: { id: attemptId },
data: {
status: DB.AdaptiveAssessmentAttemptStatus.COMPLETED,
currentTheta: nextState.theta,
currentStandardError: nextState.standardError,
finalTheta: nextState.theta,
finalStandardError: nextState.standardError,
finalLevelLabel: level?.label ?? null,
elapsedSeconds,
completedAt: new Date(),
thetaHistory: thetaHistoryForAttempt(updatedAttempt),
standardErrorHistory: standardErrorHistoryForAttempt(updatedAttempt),
},
})
} else {
await ctx.prisma.adaptiveAssessmentAttempt.update({
where: { id: attemptId },
data: {
currentTheta: nextState.theta,
currentStandardError: nextState.standardError,
thetaHistory: thetaHistoryForAttempt(updatedAttempt),
standardErrorHistory: standardErrorHistoryForAttempt(updatedAttempt),
},
})
}

return getAdaptiveAttemptState({ attemptId }, ctx)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Concurrent answer submissions can hit a unique-constraint violation

The response order is set to attempt.responses.length at the time the attempt is read (step 1). Two requests that both read the same attempt state before either commits will both compute the same order value, and the second DB write will fail with a unique-constraint violation on (attemptId, order) rather than a clean error message. In practice, adaptive tests are driven one question at a time so this is unlikely, but a retry from the client or a slow network could trigger it. Using a DB-level nextval sequence or inserting with an ON CONFLICT guard would make the order assignment concurrency-safe.

Fix in Codex Fix in Claude Code

@rschlaefli

Copy link
Copy Markdown
Member

Full review pushed to this branch: project/REVIEW-adaptive-learning-pr5113.md — covers psychometrics/didactics, security, UX, and code quality with file:line evidence, plus a phased roadmap (Phase 0 safety → Phase 3 Spanish pilot) written to be executed step by step.

Headline items before any student-facing use:

  • Solutions leak via publishedAdaptiveAssessments / adaptiveAttemptState.assessment.elements (A1/A2)
  • Save wipes all student attempts, non-atomically (A3)
  • Arbitrary/repeated answer submission allows placement manipulation (A4)
  • Theta jumps to ±3 after the first answer — MAP prior exists in the lib but is unused (B1, verified empirically)
  • Simulation validates a different level-mapping rule than production (B2, verified)

Comment on lines +800 to +824
await ctx.prisma.adaptiveAssessmentAttempt.update({
where: { id: attemptId },
data: {
status: DB.AdaptiveAssessmentAttemptStatus.COMPLETED,
currentTheta: nextState.theta,
currentStandardError: nextState.standardError,
finalTheta: nextState.theta,
finalStandardError: nextState.standardError,
finalLevelLabel: level?.label ?? null,
elapsedSeconds,
completedAt: new Date(),
thetaHistory: thetaHistoryForAttempt(updatedAttempt),
standardErrorHistory: standardErrorHistoryForAttempt(updatedAttempt),
},
})
} else {
await ctx.prisma.adaptiveAssessmentAttempt.update({
where: { id: attemptId },
data: {
currentTheta: nextState.theta,
currentStandardError: nextState.standardError,
thetaHistory: thetaHistoryForAttempt(updatedAttempt),
standardErrorHistory: standardErrorHistoryForAttempt(updatedAttempt),
},
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Attempt state race

Concurrent answer submissions can still write an incorrect attempt state. Each request creates its response, then updates currentTheta and thetaHistory from the response list it loaded before either request committed. If two different answers arrive at the same time, the last update can overwrite the attempt estimate with a history that includes only its own new response, even though both response rows now exist. The attempt state update needs to be serialized with the response insert or retried from the latest response set.

Fix in Codex Fix in Claude Code

…e-learning review

Part 5 compares the implemented item-level 3PL CAT against classification
testing (SPRT/CCT), estimator alternatives (MAP/WLE), DIALANG/Linguaskill
designs, multistage testing, comparative-judgment item leveling, and
exposure control. Verdict: keep the architecture; five concrete roadmap
adjustments in section 5.7. All citations retrieved via literature search.
@rschlaefli

Copy link
Copy Markdown
Member

Added Part 5 — literature check on approach optimality to project/REVIEW-adaptive-learning-pr5113.md (commit 1aa0f00).

TL;DR: the item-level CAT architecture is the right call — keep it. But the literature reframes a few things:

  • Placement is classification, not estimation → add a classification-aware stopping rule (stop when the θ interval sits inside one CEFR band) on top of the SE threshold; shorter tests for clear cases, more questions for boundary cases (Eggen 2011; Nydick 2014; Wang, Chen & Huebner 2021).
  • Unprimed MLE is the one estimator operational CATs avoid — the B1/MAP fix aligns with standard practice (Wang & Vispoel 1998); cross-validate the simulation against R's catR.
  • DIALANG precedent: self-assessment warm start on the intro screen (feeds initialTheta) + end-of-test result reveal.
  • Don't rebuild as MST — with level-anchored items the current design already behaves like one, with CAT's upside once empirical calibration lands.
  • Item leveling: ≥2 independent levelers + comparative judgment for contested items (Attali et al. 2014).

Concrete roadmap deltas in §5.7, full references with DOIs at the end of the file.

Comment on lines +86 to +88
return ordered.map((level, index) => {
const start = min + (span * index) / ordered.length
const end = min + (span * (index + 1)) / ordered.length

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Band boundaries still differ

This still renders level bands with span / ordered.length, while the backend assigns default NEAREST levels with theta points spread over ordered.length - 1 intervals and midpoint boundaries. With four levels over [-3, 3], this UI draws boundaries at -1.5, 0, and 1.5, but mapThetaToLevel assigns levels at -2, 0, and 2. A learner at θ=1.75 is therefore shown inside the top visual band while the backend result label still maps to the previous level, so the displayed band and assigned level can disagree.

Context Used: AGENTS.md (source)

Fix in Codex Fix in Claude Code

levelLabel: string
}

async function seedTestkursAdaptiveElements(prisma: Prisma.PrismaClient) {
return elements
}

async function seedTestkursAdaptiveAssessment(
level: Prisma.AdaptiveAssessmentLevel
}

async function seedTestkursAdaptiveAssessmentAttempts(
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants