Skip to content

Commit a1f6919

Browse files
ottenhoffottenhoff
committed
Improve review diagnostics for app capabilities
1 parent a73877d commit a1f6919

8 files changed

Lines changed: 771 additions & 4 deletions

‎src/admin/package_detail.ts‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,14 @@ function renderPendingReviewOverview(input: {
251251
previewEvidence: PreviewEvidenceRecord[];
252252
}): string {
253253
const previousVersion = resolvePreviousVersion(input.history, input.packageVersion);
254+
const previousApprovedVersion = resolvePreviousApprovedVersion(
255+
input.history,
256+
input.packageVersion,
257+
);
258+
const capabilityChanges = summarizeCapabilityChanges({
259+
current: input.capabilitySummary,
260+
previousApprovedVersion,
261+
});
254262
const sensitiveCount = input.capabilitySummary.filter((capability) => capability.flagged).length;
255263
const normalCount = input.capabilitySummary.length - sensitiveCount;
256264
const standardCapabilityText = `${normalCount} standard runtime ${
@@ -294,6 +302,7 @@ function renderPendingReviewOverview(input: {
294302
? ''
295303
: `<p class="micro muted"><a href="${diffHref}">Open the file-level version diff.</a></p>`
296304
}
305+
${renderCapabilityChangeSummary(capabilityChanges)}
297306
</article>
298307
<article class="line-item">
299308
<p class="line-title">What it can do</p>
@@ -582,6 +591,111 @@ function resolvePreviousVersion(
582591
return history[currentIndex + 1] ?? null;
583592
}
584593

594+
function resolvePreviousApprovedVersion(
595+
history: readonly PackageVersionRecord[],
596+
packageVersion: PackageVersionRecord,
597+
): PackageVersionRecord | null {
598+
const currentIndex = history.findIndex((version) => version.id === packageVersion.id);
599+
600+
if (currentIndex === -1) {
601+
return null;
602+
}
603+
604+
return history.slice(currentIndex + 1).find((version) => version.approvalStatus === 'approved') ??
605+
null;
606+
}
607+
608+
interface CapabilityChangeSummary {
609+
previousApprovedVersion: PackageVersionRecord | null;
610+
added: CapabilitySummary[];
611+
removed: CapabilitySummary[];
612+
unchanged: CapabilitySummary[];
613+
}
614+
615+
function summarizeCapabilityChanges(input: {
616+
current: CapabilitySummary[];
617+
previousApprovedVersion: PackageVersionRecord | null;
618+
}): CapabilityChangeSummary {
619+
if (input.previousApprovedVersion === null) {
620+
return {
621+
previousApprovedVersion: null,
622+
added: input.current,
623+
removed: [],
624+
unchanged: [],
625+
};
626+
}
627+
628+
const currentIds = new Set(input.current.map((capability) => capability.id));
629+
const previous = summarizeCapabilities(input.previousApprovedVersion.capabilities);
630+
const previousIds = new Set(previous.map((capability) => capability.id));
631+
632+
return {
633+
previousApprovedVersion: input.previousApprovedVersion,
634+
added: input.current.filter((capability) => !previousIds.has(capability.id)),
635+
removed: previous.filter((capability) => !currentIds.has(capability.id)),
636+
unchanged: input.current.filter((capability) => previousIds.has(capability.id)),
637+
};
638+
}
639+
640+
function renderCapabilityChangeSummary(summary: CapabilityChangeSummary): string {
641+
const addedSensitive = summary.added.filter((capability) => capability.flagged);
642+
643+
if (summary.previousApprovedVersion === null) {
644+
return `<div class="detail-stack">
645+
<p class="micro muted">Capability baseline: no previously approved version exists. Review every declared runtime capability before approval.</p>
646+
${renderCapabilityChangeGroup('Declared for first approval', summary.added)}
647+
</div>`;
648+
}
649+
650+
if (summary.added.length === 0 && summary.removed.length === 0) {
651+
return `<div class="detail-stack">
652+
<p class="micro muted">Capability changes since approved version ${
653+
escapeHtml(summary.previousApprovedVersion.version)
654+
}: no capability changes.</p>
655+
${renderCapabilityChangeGroup('Unchanged', summary.unchanged)}
656+
</div>`;
657+
}
658+
659+
return `<div class="detail-stack">
660+
<p class="micro muted">Capability changes since approved version ${
661+
escapeHtml(summary.previousApprovedVersion.version)
662+
}.</p>
663+
${renderCapabilityChangeGroup('Added', summary.added)}
664+
${renderCapabilityChangeGroup('Removed', summary.removed)}
665+
${renderCapabilityChangeGroup('Unchanged', summary.unchanged)}
666+
${
667+
addedSensitive.length === 0
668+
? ''
669+
: `<p class="micro muted">Review impact: this version newly requests ${
670+
escapeHtml(formatCapabilityNames(addedSensitive))
671+
}. Confirm the assignment purpose, evidence or grading flow, and latest preview evidence before approval.</p>`
672+
}
673+
</div>`;
674+
}
675+
676+
function renderCapabilityChangeGroup(label: string, capabilities: CapabilitySummary[]): string {
677+
if (capabilities.length === 0) {
678+
return `<p class="micro muted">${escapeHtml(label)}: none.</p>`;
679+
}
680+
681+
return `<div>
682+
<p class="micro muted">${escapeHtml(label)}</p>
683+
<ul>
684+
${
685+
capabilities
686+
.map((capability) =>
687+
`<li><strong>${escapeHtml(capability.label)}</strong>: ${escapeHtml(capability.detail)}${
688+
capability.flagged
689+
? ` <span class="micro muted">${escapeHtml(capability.sensitivityLabel)}</span>`
690+
: ''
691+
}</li>`
692+
)
693+
.join('')
694+
}
695+
</ul>
696+
</div>`;
697+
}
698+
585699
function formatChangeSummary(
586700
packageVersion: PackageVersionRecord,
587701
previousVersion: PackageVersionRecord | null,

‎src/admin/package_detail_test.ts‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ Deno.test('renderPackageDetailPage gives pending reviewers changes, launch, runt
8686
assertStringIncludes(body, 'Review before approval');
8787
assertStringIncludes(body, 'What changed');
8888
assertStringIncludes(body, 'Version 0.2.0 is pending review against previous version 0.1.0.');
89+
assertStringIncludes(
90+
body,
91+
'Capability changes since approved version 0.1.0: no capability changes.',
92+
);
93+
assertStringIncludes(body, 'Unchanged');
8994
assertStringIncludes(body, '/admin/packages/chapter-4-asteroids/versions/0.2.0/diff');
9095
assertStringIncludes(body, 'Open review test launch');
9196
assertStringIncludes(body, 'Latest review test launch preview-session-review');
@@ -98,6 +103,135 @@ Deno.test('renderPackageDetailPage gives pending reviewers changes, launch, runt
98103
assertStringIncludes(body, 'direct grade writes');
99104
});
100105

106+
Deno.test('renderPackageDetailPage treats first pending version as a full capability baseline review', () => {
107+
const pendingVersion = buildPackageVersionRecord({
108+
id: 10,
109+
version: '0.1.0',
110+
approvalStatus: 'pending',
111+
capabilities: ['read_launch_context', 'read_activity_content', 'finalize_attempt'],
112+
});
113+
const body = renderPackageDetailPage({
114+
packageVersion: pendingVersion,
115+
history: [pendingVersion],
116+
});
117+
118+
assertStringIncludes(body, 'Capability baseline: no previously approved version exists.');
119+
assertStringIncludes(body, 'Declared for first approval');
120+
assertStringIncludes(body, 'Launch context');
121+
assertStringIncludes(body, 'Reviewed app content');
122+
assertStringIncludes(body, 'Attempt completion');
123+
});
124+
125+
Deno.test('renderPackageDetailPage highlights added sensitive capabilities against the previous approved version', () => {
126+
const previousVersion = buildPackageVersionRecord({
127+
id: 20,
128+
version: '0.1.0',
129+
approvalStatus: 'approved',
130+
reviewedAt: '2026-05-15T12:00:00.000Z',
131+
capabilities: ['read_launch_context', 'read_activity_content', 'finalize_attempt'],
132+
});
133+
const pendingVersion = buildPackageVersionRecord({
134+
id: 21,
135+
version: '0.2.0',
136+
approvalStatus: 'pending',
137+
capabilities: [
138+
'read_launch_context',
139+
'read_activity_content',
140+
'submit_evidence_artifact',
141+
'finalize_attempt',
142+
],
143+
});
144+
const body = renderPackageDetailPage({
145+
packageVersion: pendingVersion,
146+
history: [pendingVersion, previousVersion],
147+
});
148+
149+
assertStringIncludes(body, 'Capability changes since approved version 0.1.0.');
150+
assertStringIncludes(body, 'Added');
151+
assertStringIncludes(body, 'Submitted evidence artifacts');
152+
assertStringIncludes(body, 'Sensitive learner evidence');
153+
assertStringIncludes(
154+
body,
155+
'Review impact: this version newly requests Submitted evidence artifacts.',
156+
);
157+
assertStringIncludes(body, 'Confirm the assignment purpose, evidence or grading flow');
158+
});
159+
160+
Deno.test('renderPackageDetailPage shows removed capabilities against the previous approved version', () => {
161+
const previousVersion = buildPackageVersionRecord({
162+
id: 30,
163+
version: '0.1.0',
164+
approvalStatus: 'approved',
165+
reviewedAt: '2026-05-15T12:00:00.000Z',
166+
capabilities: [
167+
'read_launch_context',
168+
'read_activity_content',
169+
'read_local_state',
170+
'write_local_state',
171+
'finalize_attempt',
172+
],
173+
});
174+
const pendingVersion = buildPackageVersionRecord({
175+
id: 31,
176+
version: '0.2.0',
177+
approvalStatus: 'pending',
178+
capabilities: ['read_launch_context', 'read_activity_content', 'finalize_attempt'],
179+
});
180+
const body = renderPackageDetailPage({
181+
packageVersion: pendingVersion,
182+
history: [pendingVersion, previousVersion],
183+
});
184+
185+
assertStringIncludes(body, 'Removed');
186+
assertStringIncludes(body, 'Resume saved progress');
187+
assertStringIncludes(body, 'Save resumable progress');
188+
assertStringIncludes(body, 'Added: none.');
189+
});
190+
191+
Deno.test('renderPackageDetailPage ignores newer approved versions when reviewing an older pending version', () => {
192+
const newerApprovedVersion = buildPackageVersionRecord({
193+
id: 40,
194+
version: '0.3.0',
195+
approvalStatus: 'approved',
196+
reviewedAt: '2026-06-15T12:00:00.000Z',
197+
importedAt: '2026-06-14T12:00:00.000Z',
198+
capabilities: [
199+
'read_launch_context',
200+
'read_activity_content',
201+
'submit_evidence_artifact',
202+
'finalize_attempt',
203+
],
204+
});
205+
const pendingVersion = buildPackageVersionRecord({
206+
id: 41,
207+
version: '0.2.0',
208+
approvalStatus: 'pending',
209+
importedAt: '2026-06-01T12:00:00.000Z',
210+
capabilities: [
211+
'read_launch_context',
212+
'read_activity_content',
213+
'submit_evidence_artifact',
214+
'finalize_attempt',
215+
],
216+
});
217+
const olderApprovedVersion = buildPackageVersionRecord({
218+
id: 42,
219+
version: '0.1.0',
220+
approvalStatus: 'approved',
221+
reviewedAt: '2026-05-15T12:00:00.000Z',
222+
importedAt: '2026-05-14T12:00:00.000Z',
223+
capabilities: ['read_launch_context', 'read_activity_content', 'finalize_attempt'],
224+
});
225+
const body = renderPackageDetailPage({
226+
packageVersion: pendingVersion,
227+
history: [newerApprovedVersion, pendingVersion, olderApprovedVersion],
228+
});
229+
230+
assertStringIncludes(body, 'Capability changes since approved version 0.1.0.');
231+
assertStringIncludes(body, 'Submitted evidence artifacts');
232+
assertStringIncludes(body, 'Sensitive learner evidence');
233+
});
234+
101235
Deno.test('renderPackageDetailPage distinguishes ordinary progress from sensitive evidence and saved file details', () => {
102236
const reviewedVersion = buildPackageVersionRecord({
103237
approvalStatus: 'approved',

0 commit comments

Comments
 (0)