feat(picker): normalize aliases + Choices.js tag picker - #183
Merged
martinyde merged 13 commits intoJul 7, 2026
Merged
Conversation
Server-side, AssistantCreator now folds submitted language-model values (including aliases like openai/gpt-4o) through ModelMap to their canonical id before persisting, keeping the catalogue's facet deduplicated regardless of the spelling a curator typed. Client-side, a Stimulus controller enhances the free-text languageModel input with Choices.js in text-input mode. The dropdown offers the union of ModelMap::choices() and every languageModel value already in the catalogue (case-insensitive dedup, canonical spelling wins on ties), and aliases from model_map.yaml are exposed on each choice's customProperties so Fuse.js filters the dropdown as the curator types any known spelling. Free-typed values commit as-is via addItems and the new addItemText callback, and the pill preserves whatever the curator typed. The datalist fallback survives for no-JS clients. New AssistantRepository::persistedLanguageModels() feeds the union list, and new ModelMap::aliasesFor() exposes the per-canonical alias tokens the picker searches on.
The previous commit ran Choices.js in text-input mode on the languageModel `<input>` and seeded choices via setChoices() — Choices.js rejects that combination at runtime with "setChoices cannot be used with input based choices", breaking `/assistant/new` step 2 for every visitor. Render the field as a `<select>` in the template (manual render so the field stays TextType server-side and free-typed values still round-trip verbatim). The `<option>` list is the same union AssistantMetadataStepType::finishView() already exposes via `model_choices` — canonical shortlist plus every persisted value, case-insensitive dedup, canonical spelling wins ties. The controller now wraps the `<select>` in Choices.js's select-one search combobox with `searchChoices: false` and owns the `search` event: on every keystroke it emits a filtered list matching value / label / any alias from model_map.yaml (typing `openai/gpt` narrows to `gpt-4o`), and appends the raw typed value as a pickable row when it isn't already a known option. On `choice`, freetagged commits get promoted to the known set so subsequent keystrokes don't duplicate them. Pill still shows whatever the curator typed. Drops the now-unused MODEL_DATALIST_ID const, the `list` attribute on the field, and the datalist markup in the template. No-JS clients keep a plain `<select>` populated with the same option list.
…ect loop With the controller bound to the `<select>` itself, Choices.js re-parenting the select into its own DOM wrapper on init took the controller node out of the live tree — Stimulus fired disconnect, then reconnect once Choices.js re-inserted it, running `new Choices()` again, and so on in a loop. Move `data-controller="language-model-picker"` and its values onto a stable wrapper `<div>` around the `<select>`, and look the select up with `querySelector` inside `connect()`. The wrapper never leaves the tree, so the controller initialises exactly once.
… legend Three visible glitches on `/assistant/new` step 2: - `Sprogmodel` rendered twice. The template renders the language-model widget manually as a `<select>` (so Choices.js can enhance it) but never marked the underlying form child as rendered — `form_end()` then emitted the auto-widget with its own label a second time. Mark the child via `setRendered` after the manual render. - A dangling "Metadata" legend above the fields. Same root cause: `form_end()` was seeing the compound step form with one unrendered leaf and emitting the whole thing (label + legend + unrendered leaf). Fixed by the setRendered above — with every leaf rendered, the compound emits nothing. - Redundant "Annuller" link + inline previous/next buttons. Dropped the link and moved the navigator into a right-aligned button row rendered explicitly, so we can style each button (previous → subdued outline, next / finish → primary). Each button self-hides via NavigatorFlowType's `include_if` callback — `previous is defined` guards keep the render safe on the first / last steps. Previous-button label changed to "Indsæt anden JSON" — clearer than "← Forrige trin" now that the button is on the metadata step and its only effect is to go back to the JSON entry step. Dropped the now-unused `assistant.new.action.cancel` translation key.
…n 1-2 Step 3 (receipt) now surfaces three actions inline instead of two: "Gå til assistent" (button-styled anchor to the new entity), "Del en til" (the flow's Finish submit, which resets session state and lands back on step 1), and "Tilbage til kataloget" (text link to the catalog). The three sit in a left-aligned row inside the receipt partial itself so the anchor links can live alongside the Finish submit without needing a wrapper form. The parent shell (`new.html.twig`) skips its own navigator row on the receipt step and passes `flow` into the include so the Finish widget can be rendered inline. `setRendered` on the whole navigator keeps `form_end` from re-emitting the still-defined-but-unused `previous` button below the actions row. Steps 1 and 2 keep their right-aligned Previous / Next pair (previous only appears on step 2 per `include_if`).
The Finish button on step 3 was wired to `assistant.new.step_receipt.share_another`, but the actual translation lives under `assistant.new.action.share_another` — the receipt page missed the translation and rendered the raw key. Point the label at the correct path.
`AssistantReceiptStepType` is a render-only step with no fields of its own. `FormView::isRendered()` on a childless compound just returns its own `$rendered` flag (nothing to walk into), so `form_end` at the bottom of `new.html.twig` would auto-emit the compound with its default humanized legend — "Receipt" — below the actions row. Mark the compound rendered explicitly in the receipt partial to suppress it, alongside the existing `flow.navigator.setRendered` that handles the leftover previous button.
Symfony's Validator component translates constraint messages via the `validators` translation domain by default. The wizard's five constraint messages (`step_json.required` + `step_json.invalid_config`, plus `title_required`, `description_required`, `language_model_required` on `step_metadata`) lived in `messages.da.yaml` — so `form_errors()` looked them up in the wrong catalogue and rendered the raw keys. Move all five entries to `translations/validators.da.yaml` under the same nested paths. `step_json.unrecognised_format` stays in `messages.da.yaml` — it's rendered via `$translator->trans(...)` from the controller (default domain = `messages`), not via a violation.
…ion family
The step-1 config validator now emits one violation per deduped
detail line instead of a single violation with a `; `-joined
`{{ errors }}` placeholder. Symfony's default `form_errors`
renders the sequence as a real `<ul>`, so the user sees a
bulleted list of reasons rather than a semicolon-separated
paragraph.
The first violation is the localised intro line
(`assistant.new.step_json.invalid_config` →
"Filen er ikke en gyldig assistent-konfiguration:") from the
existing `validators` catalogue; each following violation is
one detail line.
Each detail line is passed through a new
`assistant_validation` translation domain
(`translations/assistant_validation.da.yaml`) so the finite
JSON-decoder message family (PHP's `\JsonException`) can be
localised without cluttering the general validator catalogue.
Opis JsonSchema errors carry interpolated `{property}`/`{path}`
context — they fall through the catalogue as English rather
than requiring a per-keyword mapping that would drift with
library bumps.
Test updates: `testSchemaInvalidRaisesViolation` and
`testMalformedInputRaisesViolation` now chain `buildNextViolation`
per detail line to match the new shape. `createValidator` stubs
a passthrough `TranslatorInterface` so the assertions keep
matching the raw adapter error strings.
Tailwind's preflight strips `list-style` and left padding from every `<ul>`, so the config-validator's per-error list rendered as a column of naked text lines inside the alert box. Register an inline `_self` form theme on the wizard form that overrides `form_errors` with `list-disc space-y-1 pl-5`. Scoped to `new.html.twig` — no site-wide styling change. Also applies to the language-model field's inline `form_errors(language_model)` on step 2, which is the same styling.
…leted list Two glitches on step 1 when the config validator fires: - Errors rendered twice — once via `form_errors(flow)` in the wizard's alert box and again next to the textarea via `form_row(flow.json.sourceConfig)`. The violations attach to the `sourceConfig` field by default, so both spots pulled from the same list and the user saw duplicates. - The bulleted list styling from the earlier `_self` form-theme override didn't take effect. Tailwind's preflight strips `list-style` on every `<ul>`, and the override was competing with existing form theming rather than owning the render. Fixes: - `error_bubbling: true` on the `sourceConfig` field. The parent step has `inherit_data: true`, whose default `error_bubbling` is true — so the errors climb the compound and land on the flow root. `form_row(...)` on the field no longer sees any attached errors and stops rendering the duplicate row. - Drop the `_self` form-theme override; render the alert box contents directly in `new.html.twig`. First error (the intro "Filen er ikke en gyldig assistent-konfiguration:") is a `<p>` heading. Remaining errors become a `<ul>` with `list-disc space-y-1 pl-5` classes, which bypass Tailwind's preflight because the utilities set `list-style-type` explicitly. No test changes — the validator's violation shape is unaffected; only the field's `error_bubbling` option and the wizard shell's render change.
Two adjustments to the wizard's error box: - Drop the "first error is a summary" treatment. Any error in the list could arrive on its own (there is no guarantee the config-validator's intro line is always first once other validators contribute), so treating the first entry as a paragraph heading and the rest as bullets is inconsistent. Render every error as a plain `<li>` bullet. - Use the shared `<twig:Alert type="error">` component instead of hand-rolling the alert `<div>`. Same visual today, but any future colour/icon treatment on the component reaches this spot for free. Bullets are re-styled with `list-disc pl-5` and `marker:text-text` so the disc glyphs actually show through Tailwind's preflight and match the surrounding muted text.
Tailwind's preflight sets `list-style: none` on every `<ul>` in the project's base CSS, and the `list-disc` utility couldn't overcome it in this spot. Bullets never showed. Drop `list-disc` entirely; render the bullet as a `::before` pseudo-element on each `<li>` with `content-['•']` positioned absolutely at `left-0` inside a `pl-4` inset. Coloured via `before:text-text` so the glyph matches the surrounding muted body copy. Unaffected by any `list-style` reset upstream.
martinyde
approved these changes
Jul 7, 2026
martinyde
merged commit Jul 7, 2026
8790bfc
into
feature/issue-177-language-model-picker
8 checks passed
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Links to issues
Refs #177.
Description
Layered onto PR #178. Two orthogonal picks:
AssistantCreatornowruns the submitted
languageModelthroughModelMap::normalise()before construction, so aliases (e.g.openai/gpt-4o,gpt-4o-2024-08-06) fold to the canonical id(
gpt-4o) at persist time. Unknown / free-typed values passthrough untouched. Keeps the catalog's language-model facet
deduplicated no matter which spelling a curator submitted.
enhances the free-text
languageModelinput with Choices.jsin text-input mode (
maxItemCount: 1,addItems: true). Thedropdown is seeded with the union of
ModelMap::choices()and
AssistantRepository::persistedLanguageModels()(case-insensitive dedup, canonical spelling wins ties).
Aliases from
model_map.yamlcome through on each choice'scustomProperties.aliasesso Fuse.js filters as the curatortypes any known spelling — typing
openai/gptnarrows thedropdown to
gpt-4o. The pill preserves whatever the curatortyped; normalisation only happens on the server.
Datalist fallback for no-JS clients is unchanged.
Primary files:
src/Assistant/AssistantCreator.php— normalise on persist.src/Assistant/Model/ModelMap.php— newaliasesFor().src/Repository/AssistantRepository.php— newpersistedLanguageModels().src/Form/AssistantMetadataStepType.php— widenfinishView()to expose union + aliases.
assets/controllers/language_model_picker_controller.js—Choices.js text-input mode.
templates/assistant/_new_step_metadata.html.twig— wire thecontroller onto the field.
Screenshot of the result
To be added — screenshots of the picker (canonical + persisted
dropdown, alias-search filtering, free-tag entry) will follow
after manual smoke on the review env.
Checklist
Additional comments or questions
Client-side normalise-on-commit (Option 3) is deliberately not
in this PR — deferred pending manual test of the current
"typed value stays visible in the pill" behaviour.
Details - AI specificities
Normalisation shape.
AssistantCreator::create()now folds$languageModelvia$this->modelMap->normalise($languageModel) ?? $languageModelright before instantiating the
Assistant. Passing through thenullfallback preserves free-typed values verbatim (thefallback: passthroughpolicyModelMapdocuments). TheAssistantDraftPrefilleralready normalises on the importpath, so the two persistence entry points now agree.
Union merge shape.
AssistantMetadataStepType::finishView()buildsmodel_choicesas
label => id:ModelMap::choices()— canonical shortlist, inmap order.
set; append it to the choices when it's not already covered.
Canonical spelling wins on ties.
Aliases are exposed as a parallel
model_aliasesmap(
id => list<string>), the source of truthModelMapreturns via the new
aliasesFor()accessor.Client controller shape.
language_model_picker_controller.jsis a thin Choices.jswrapper:
<input type="text">, so theunderlying form element and its submitted name don't change.
maxItemCount: 1+addItems: true— single pill,free-tags allowed.
searchFields: ['label', 'value', 'customProperties.aliases']— Fuse.js indexes the canonical id, its label, and the
space-joined alias tokens for each row.
setChoices()is called post-init with the union list Twigencoded into a
data-language-model-picker-choices-valueattribute.
Pill preserves typed value.
Per the user's preference (
openai/gpt-4ois what curators areused to seeing), the pill shows whatever the curator typed.
Server-side normalise is what folds the value to
gpt-4oforstorage; the two-way discrepancy is intentional.
Test additions.
AssistantCreatorTest— one test provingopenai/gpt-4o→gpt-4oon persist, one proving unknownvalues (
my-local-llm) survive verbatim.AssistantRepositoryTest— a newtestPersistedLanguageModelsListsDistinctStoredValuestest.ModelMapTest— newtestAliasesForReturnsDeclaredAliasesOnlytest.
AssistantMetadataStepTypeTest— expanded to cover thecanonical+aliases exposition, the persisted-extras append,
and the case-insensitive dedup with canonical winning.
Coverage. 565 tests, 1674 assertions, 100.00 % on the
coverage gate.
Related.
ModelMapservice introduced in PR feat: format-agnostic import/export with four new adapters #179 (cross-format adapters +
config/model_map.yaml).