Skip to content

Commit c2f1a19

Browse files
fix(acp): preserve enum values with distinct option labels
Adapt the enum labeling from pingdotgg#31081 to the typed form parser. Keep empty and padded values exact and disambiguate trimmed labels, including collisions with literal quoted values. Co-authored-by: Honey Paptan <hpaptan@gmail.com>
1 parent 4dddeb4 commit c2f1a19

2 files changed

Lines changed: 70 additions & 8 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { assert, describe, it } from "@effect/vitest";
2+
3+
import { parseElicitationForm } from "./elicitationForm.ts";
4+
5+
describe("parseElicitationForm", () => {
6+
for (const type of ["string", "array"] as const) {
7+
it.each([
8+
{ values: [" padded ", "", "plain"], labels: ["padded", '""', "plain"] },
9+
{ values: [" a", "a", "b"], labels: ['" a"', '"a"', "b"] },
10+
{ values: [" a", "a", '"a"'], labels: ['" a"', '"a"', '"\\"a\\""'] },
11+
])(
12+
`preserves ${type} enum values with distinct readable labels: $values`,
13+
({ values, labels }) => {
14+
const form = parseElicitationForm(
15+
{
16+
type: "object",
17+
properties: {
18+
answer:
19+
type === "array"
20+
? { type, items: { type: "string", enum: values } }
21+
: { type, enum: values },
22+
},
23+
required: ["answer"],
24+
},
25+
"Choose",
26+
);
27+
assert(form !== undefined);
28+
const question = form.questions[0];
29+
assert(question !== undefined);
30+
assert.deepStrictEqual(
31+
question.options.map((option) => option.label),
32+
labels,
33+
);
34+
assert.deepStrictEqual(
35+
question.options.map((option) => option.value),
36+
values,
37+
);
38+
assert.strictEqual(question.allowCustomAnswer, false);
39+
assert.strictEqual(question.multiSelect, type === "array" ? true : undefined);
40+
for (const value of values) {
41+
const answer = type === "array" ? [value] : value;
42+
assert.deepStrictEqual(form.content({ answer }), { answer });
43+
}
44+
},
45+
);
46+
}
47+
});

‎packages/provider-acp/src/server/elicitationForm.ts‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,27 @@ function choiceOptions(
4646
return options.every((option) => option !== undefined) ? options : undefined;
4747
}
4848

49-
/** `enum` values label themselves. */
50-
function enumOption(value: unknown): Option | undefined {
51-
const label = text(value);
52-
return typeof value === "string" && label !== undefined
53-
? { label, description: label, value }
54-
: undefined;
49+
function enumOptions(entries: unknown): Option[] | undefined {
50+
if (
51+
!Array.isArray(entries) ||
52+
entries.length === 0 ||
53+
!entries.every((value): value is string => typeof value === "string")
54+
)
55+
return undefined;
56+
const counts = new Map<string, number>();
57+
for (const value of entries) counts.set(value.trim(), (counts.get(value.trim()) ?? 0) + 1);
58+
const options = entries.map((value) => {
59+
const plain = value.trim();
60+
const label = plain !== "" && counts.get(plain) === 1 ? plain : JSON.stringify(value);
61+
return { label, description: label, value };
62+
});
63+
// A quoted fallback can collide with a literal label; quote every value in that case.
64+
return new Set(options.map((option) => option.label)).size === options.length
65+
? options
66+
: options.map((option) => {
67+
const label = JSON.stringify(option.value);
68+
return { ...option, label, description: label };
69+
});
5570
}
5671

5772
/** Titled entries (`oneOf` on strings, `anyOf` on array items) carry a title for their `const`. */
@@ -74,7 +89,7 @@ function parseField(
7489
}
7590
const options = Array.isArray(property.oneOf)
7691
? choiceOptions(property.oneOf, titledOption)
77-
: choiceOptions(property.enum, enumOption);
92+
: enumOptions(property.enum);
7893
return options === undefined
7994
? undefined
8095
: {
@@ -87,7 +102,7 @@ function parseField(
87102
const items = unknownRecord(property.items);
88103
const options =
89104
items?.type === "string"
90-
? choiceOptions(items.enum, enumOption)
105+
? enumOptions(items.enum)
91106
: items !== undefined && items.type === undefined
92107
? choiceOptions(items.anyOf, titledOption)
93108
: undefined;

0 commit comments

Comments
 (0)