refactor: papercut cleanup before per-question-type bundle

Two small cleanups that calm the diff for the upcoming §3.A per-question-
type module migration:

1. Rename setScaleLabel → setDateRankedScaleLabel in FormBuilder.svelte.
   The function only handles date_ranked_choice's nested scale.{min,max}_label
   (early-returns on every other type); the standalone scale question type's
   labels use inline handlers. The original name was a half-grown helper from
   the m/fdbck#1 add. The rename removes a real "wait, why does this skip
   scale questions?" papercut that the audit doc flagged.

2. results.ts: replace the (s as ScaleStats & { _sum?: number })._sum
   inline-cast-and-coalesce pattern with proper ScaleStatsWip /
   DateRankedOptionStatsWip accumulator types. _sum is now a real, typed
   field during ingest; finalise reads it and drops it before returning the
   public stats shape. No behaviour change, no new tests needed — existing
   results.test.ts still passes.

   Also: kept `void q;` but expanded the comment to explain that q is the
   schema definition (kept in signature for the upcoming per-type module
   refactor that will dispatch via q.type).

54 tests pass, svelte-check clean, build clean.
This commit is contained in:
mAi
2026-05-07 19:58:12 +02:00
parent 67cc71a6be
commit 37cb874096
2 changed files with 45 additions and 29 deletions

View File

@@ -175,7 +175,7 @@
update(idx, { options: q.options.filter((_, i) => i !== optIdx) } as Partial<FeedbackQuestion>);
}
function setScaleLabel(idx: number, which: 'min_label' | 'max_label', val: string): void {
function setDateRankedScaleLabel(idx: number, which: 'min_label' | 'max_label', val: string): void {
const q = value.questions[idx];
if (q.type !== 'date_ranked_choice') return;
const scale = { ...(q.scale ?? {}) };
@@ -390,7 +390,7 @@
maxlength="50"
placeholder="e.g. doesn't work"
value={q.scale?.min_label ?? ''}
oninput={(e) => setScaleLabel(i, 'min_label', (e.target as HTMLInputElement).value)}
oninput={(e) => setDateRankedScaleLabel(i, 'min_label', (e.target as HTMLInputElement).value)}
/>
</div>
<div class="fb-question">
@@ -400,7 +400,7 @@
maxlength="50"
placeholder="e.g. works great"
value={q.scale?.max_label ?? ''}
oninput={(e) => setScaleLabel(i, 'max_label', (e.target as HTMLInputElement).value)}
oninput={(e) => setDateRankedScaleLabel(i, 'max_label', (e.target as HTMLInputElement).value)}
/>
</div>
</div>

View File

@@ -102,10 +102,17 @@ export function aggregateResults(
return { total_submissions: subs.length, questions };
}
// Aggregation-time accumulator types. The `_sum` field is a real, typed
// member during ingest; finalise reads it and drops it before returning the
// public stats shape. Cleaner than the previous (s as X & { _sum?: number })
// inline-cast-and-coalesce pattern.
type ScaleStatsWip = ScaleStats & { _sum: number };
type DateRankedOptionStatsWip = DateRankedOptionStats & { _sum: number };
function emptyStats(q: FeedbackQuestion): QuestionStats {
switch (q.type) {
case 'scale':
return {
case 'scale': {
const wip: ScaleStatsWip = {
type: 'scale',
count: 0,
min: q.min,
@@ -115,7 +122,10 @@ function emptyStats(q: FeedbackQuestion): QuestionStats {
value: q.min + i,
count: 0,
})),
_sum: 0,
};
return wip;
}
case 'single_choice':
case 'multi_choice':
return {
@@ -133,15 +143,19 @@ function emptyStats(q: FeedbackQuestion): QuestionStats {
return {
type: 'date_ranked_choice',
count: 0,
options: q.options.map((opt) => ({
id: opt.id,
start: opt.start,
end: opt.end ?? null,
label: opt.label ?? null,
count: 0,
mean: null,
histogram: [1, 2, 3, 4, 5].map((value) => ({ value, count: 0 })),
})),
options: q.options.map((opt) => {
const wip: DateRankedOptionStatsWip = {
id: opt.id,
start: opt.start,
end: opt.end ?? null,
label: opt.label ?? null,
count: 0,
mean: null,
histogram: [1, 2, 3, 4, 5].map((value) => ({ value, count: 0 })),
_sum: 0,
};
return wip;
}),
};
}
}
@@ -156,11 +170,11 @@ function ingest(
switch (s.type) {
case 'scale': {
if (typeof v !== 'number' || !Number.isFinite(v)) return;
s.count++;
const bucket = s.histogram.find((b) => b.value === v);
const acc = s as ScaleStatsWip;
acc.count++;
const bucket = acc.histogram.find((b) => b.value === v);
if (bucket) bucket.count++;
(s as ScaleStats & { _sum?: number })._sum =
((s as ScaleStats & { _sum?: number })._sum ?? 0) + v;
acc._sum += v;
return;
}
case 'single_choice': {
@@ -205,32 +219,34 @@ function ingest(
const raw = ratings[opt.id];
if (raw === undefined || raw === null) continue;
if (typeof raw !== 'number' || !Number.isInteger(raw) || raw < 1 || raw > 5) continue;
opt.count++;
const bucket = opt.histogram.find((b) => b.value === raw);
const acc = opt as DateRankedOptionStatsWip;
acc.count++;
const bucket = acc.histogram.find((b) => b.value === raw);
if (bucket) bucket.count++;
(opt as DateRankedOptionStats & { _sum?: number })._sum =
((opt as DateRankedOptionStats & { _sum?: number })._sum ?? 0) + raw;
acc._sum += raw;
touched = true;
}
if (touched) s.count++;
return;
}
}
void q; // unused after switch covers all branches
void q; // q is the schema definition; ingest dispatches on s.type which
// is derived from it via emptyStats. Kept in the signature so a
// future per-type module can take the question for richer logic.
}
function finalise(s: QuestionStats): void {
if (s.type === 'scale') {
const sum = (s as ScaleStats & { _sum?: number })._sum;
s.mean = s.count > 0 && typeof sum === 'number' ? sum / s.count : null;
delete (s as ScaleStats & { _sum?: number })._sum;
const acc = s as ScaleStatsWip;
s.mean = s.count > 0 ? acc._sum / s.count : null;
delete (s as Partial<ScaleStatsWip>)._sum;
return;
}
if (s.type === 'date_ranked_choice') {
for (const opt of s.options) {
const sum = (opt as DateRankedOptionStats & { _sum?: number })._sum;
opt.mean = opt.count > 0 && typeof sum === 'number' ? sum / opt.count : null;
delete (opt as DateRankedOptionStats & { _sum?: number })._sum;
const acc = opt as DateRankedOptionStatsWip;
opt.mean = opt.count > 0 ? acc._sum / opt.count : null;
delete (opt as Partial<DateRankedOptionStatsWip>)._sum;
}
// Sort by mean desc with tiebreaks: count of "5"s, then "4"s, then count desc, then id.
s.options.sort((a, b) => {