Conversation
AES-256-GCM, key derived from SESSION_SECRET via SHA-256. Format v1:iv:tag:ciphertext, all base64. decryptSecret returns '' on any malformed/tampered/empty input instead of throwing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ironment Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GET/POST /admin/instance/ai to read and save settings, POST /admin/instance/ai/test for a real round-trip so a bad key surfaces immediately. Panel greys out and shows a notice when AI_* env vars are in control. Key never rendered back to the browser, only its hint. Manual verification (Step 5 of the task brief) not run in this sandbox — no docker socket access. Verify live via make dev. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The enable switch is the one field a user expects to take effect the moment they flip it. Every other field still needs Save.
GET /admin/instance/ai returns the *resolved* base URL (the current preset's default already filled in) for display. The panel's baseUrl field carries that value even when hidden for a non-custom provider, and every save sent it back verbatim. saveAiSettings then persisted it as an explicit override, which from that point on shadowed every other provider's own endpoint on every future read. Concretely: the first-ever save (any provider, even just the enable toggle) pinned baseUrl to OpenRouter's URL. Switching the provider to Anthropic afterward kept silently sending requests to OpenRouter, surfacing as an OpenRouter-side auth error that looked like Anthropic had rejected the key. normalizeStoredBaseUrl() only lets a save persist a baseUrl override for the custom provider, where the field is actually user-edited.
…i model - The provider dropdown's change handler only ever updated the text model; defaultVisionModel wasn't even sent by GET /admin/instance/ai, so the vision model field was stuck on whatever the previous provider had left there. - 'Test connection' tested the saved config, so a freshly typed/pasted key couldn't be tried without committing it to Save first. The route now accepts the panel's current fields (resolveTestConfig), falling back field-by-field to what's stored - same 'blank means unchanged' convention the save endpoint already uses for the key. Shared the env-override + preset-fallback logic between resolveAiConfig and resolveTestConfig via a new resolveCandidate() rather than duplicating it. - gemini-2.0-flash is retired; Google's own 404 points at gemini-3.6-flash, confirmed working and set as the new default for both fields.
…ight Matches the fa-spinner fa-spin + disabled pattern already used for the other long-running admin actions (metadata refresh, gravatar import).
Applies to dvd and games only (the plugins that declare supportsBarcodeSearch) - music's barcode scan is searched directly against Discogs, which is barcode-aware in its own indexing, so it never needed this UPC-lookup step to begin with. Verified live against dvinyl_app on worktree-ai-assist: - AI off: identical to before (barcode_not_found), confirmed the branch is entered (UPC lookup genuinely returns null for the test code) and produces the unchanged render. - AI on, code with no genuine match: the model declines rather than guess (2.2s round-trip vs 454ms baseline, confirming it actually ran), same graceful barcode_not_found. - AI on, a real UPC: resolveBarcodeWithAi returns a proper 'Title Creator' query from a live provider round-trip.
Music's own barcode scan searched Discogs directly with the raw digits
and never used DVinyl's UPC-lookup step, unlike dvd/games. Turning
supportsBarcodeSearch on brings it the same AI fallback when UPCitemdb
finds nothing, with a noise-term list mirroring dvd/games' so a raw
retail title ("Discovery Vinyl LP Reissue Gatefold") gets cleaned to
a usable Discogs query the same way.
Verified live: an invalid-checksum EAN on /add-music now enters the
barcode branch (previously it fell straight through as a literal text
search) and a 1550ms round-trip confirms the AI fallback genuinely ran.
The AI fallback can add a couple of seconds to what was normally an instant UPC lookup. It's a plain form POST (full navigation, not fetch), so the spinner just rides out the page load - one listener on the shared search form covers a typed query, a Search click, and a scan finding a code. Switched the scan-success auto-submit from form.submit() to form.requestSubmit(): the former does not fire the 'submit' event at all, which would have silently skipped the spinner on exactly the path most likely to hit the AI fallback. Verified the listener directly (dispatched a blocked submit event to avoid racing real navigation): button becomes disabled, spinner icon, 'Searching...' text.
Modelled on csvImportRoute.ts: /import/ai/preview extracts and validates rows via extractRows(), nothing saved; /import/ai re-validates the reviewed rows server-side (never trusts the POST body) and hands them to runCsvImport() under an identity mapping, so they inherit its deduplication, enrichment and progress events. runCsvImport() now accepts pre-parsed CsvImportSpec.rows as an alternative to req.body.csv - the AI route's rows are already records, just not from a file. Verified live: POST /import/ai/preview with a real book list returns title/author filled and 0 missingRequired for both rows, plus bonus fields (year, publisher) the model picked up on its own; 0 items written to the DB, confirming the preview step saves nothing.
Mirrors admin-csv-import.ejs's structure and progress-socket handling;
only the first step differs (a pasted list through /import/ai/preview
instead of a file through /import/csv/preview). One column per field
the AI actually populated on at least one row, not every field the
plugin could accept - a book plugin alone has twenty-odd fields, and a
short text list rarely fills more than a handful.
Whether AI is configured is decided server-side (GET /admin already
computes it via getAiConfig(), no reason to make the browser ask
again) and threaded through as a plain aiConfigured local, guarded
with locals.aiConfigured in the partial since two of the three routes
that render this page don't set it.
Verified live end-to-end with a 5-book list:
- review table shows the right columns, all rows checked by default
- edited a cell (Hobbit's year) - the edited value reached the saved
item, not the original AI guess
- unticked a row (Neuromancer) - correctly absent from the DB after import
- 'Dune' collided with a pre-existing seeded item and was not
duplicated - runCsvImport's own deduplication running unmodified on
AI-sourced rows, exactly as designed
- enrichment was genuinely attempted (4 real Hardcover lookups in the
server log) and failed only on a missing HARDCOVER_API_KEY in this
local test env - not a defect, the same class of gap as the earlier
missing DISCOGS_TOKEN
- deliberate nonsense input produced a clean empty table, no crash
Also fixed the back button in ai-review.ejs, which had been reusing
admin.csv_import.btn_back ('Change file') though this flow has no
file - added ai.import_back instead.
'charli xcx brat and dominic fike sunburn' (real report) returned an empty row list. Traced with the raw model reply logged: the model itself replied '[]' - not a parsing bug, the prompt was making it decline. Reproduced deterministically (3x): the same content phrased as 'artist - title, artist - title' extracted correctly; the identical content as '...and...', newline-separated, or even properly capitalized dash-separated all came back empty. The prompt's 'never invent... empty is always better than a guess' wording, with no explicit permission for casual phrasing, was making the model treat uncertainty about *any* attribute as grounds to drop the whole item - even when the title and artist were plainly present in the text. Added an explicit line that identifying an item from the input is not inventing, only fabricating unknown attributes (year, ISBN, catalog number) is, plus a line permitting informal phrasing (comma/'and'/ newline-separated, one item per line or several in one line). Verified against the exact reported input directly through /import/ai/preview: now extracts both items correctly. Existing tests still pass unchanged - they assert on 'JSON array', 'title' and 'empty string' appearing in the system message, all still present.
Reported: AI-imported items just kept the model's own values instead
of getting enriched from Discogs/Hardcover. Root cause: aiImportRoute
gave runCsvImport no searchQuery, so it fell back to runCsvImport's
own generic default - the title alone. For a plugin with a common
one-word title (an album called 'Cross', a song called 'Sunburn'),
that's ambiguous enough to miss the right release entirely; each
plugin's own dedicated CSV importer already knows to combine the
creator with the title for exactly this reason.
Rather than hardcoding that combination as the AI import's default -
which would also apply it to dvd/games, whose own importers
deliberately search by title alone, presumably because TMDB/IGDB
aren't proven to tolerate an extra name in the query - added
includeCreatorInSearch to PluginDefinition so each plugin states its
own already-known-correct answer, and only music and books opt in
(matching their existing importers exactly). Verified live: 'Cross' by
Justice now pulls real Discogs data (cover, 2007, Ed Banger Records,
genres, discogs_id); 'The Matrix' still searches TMDB by title alone,
confirmed by the unchanged log line ('Enrichment failed for "The
Matrix"', no director text).
Reported: '2 lines skipped on error'. Traced to the server log: Music validation failed: quantity: Path `quantity` (0) is less than minimum allowed value (1) - for both rows. The model reliably writes a literal 0 for a count the input never stated (nobody lists how many copies of an album they own), and 0 is mongoose's own quantity default *unless a value is explicitly set*, so this bare 0 skips the schema default(1) and fails the min:1 validator outright, taking the whole row down with it. Corrected in validateRows() - it already owns exactly this class of guarantee - to 1, shown plainly in the review table rather than left blank to rely on an invisible DB fallback. Scoped to the field named 'quantity' specifically: it's the one field in this app's shared schema (models/Item.ts) whose valid range starts at 1; every other number field (rating, pages...) permits 0 as a real value, confirmed by checking every plugin's schema (rating is min:0, max:5), so this is not generalised to '0 means unknown' for numbers at large. Verified against the exact reported input end to end: both rows now save without error and pick up real Discogs enrichment.
Reported: 'charli xcx' saved as typed instead of 'Charli XCX'. The prompt never said anything about capitalization, so the model's behavior was unspecified - sometimes it normalized, sometimes it echoed the input's literal casing verbatim. Added an explicit rule to write titles and creator names in their real, correctly capitalised form, framed as normalizing an identified name rather than inventing information (so it doesn't reopen the 'declining items' issue fixed earlier). Verified: 'charli xcx' -> 'Charli XCX', 'dominic fike' -> 'Dominic Fike', 3/3 consistent against the reported input (2/2 lowercase without the rule). Also confirmed against 'daft punk discovery, the strokes is this it' -> 'Daft Punk'/'Discovery', 'The Strokes'/'Is This It', both fields correct. Note: 'brat' stays lowercase - that's Charli XCX's own deliberate official stylization, not a residual bug.
A shared window.showToast() (views/partials/toast.ejs, included once from admin.ejs) replaces the native alert() both import flows used to report their result - the AI import and the sibling CSV import, which had the exact same call. Bottom-right, auto-dismisses after 5s (shown by a shrinking bar along its bottom edge, same duration driving both the CSS animation and the JS timer so they can't drift apart) or immediately on click. The old alert() blocked until 'OK', so window.location.reload() only ran once the user had dismissed it and presumably read the result. A toast doesn't block, so onDismiss preserves that same ordering - reload fires when the toast is actually dismissed (click or the 5s timer), not the instant it appears. Verified live: correct position, bar shrinks linearly in step with the 5s timer (measured at 1.5s/3s), auto-dismiss and click-dismiss both fire onDismiss correctly. Falls back to the old alert()+reload if showToast is somehow unavailable.
An AI-extracted line like "The Matrix movie" has no way to supply a director, and the schema's required:true blocked the preview row before TMDB enrichment ever ran. Movies now save with an empty director when unknown; the existing toJSON fallback already displays that as 'Unknown'.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> - aiImportRoute.ts: accept an `images` array of already-downscaled data URIs in the preview step alongside (or instead of) pasted text; switch to the vision model and a longer timeout when images are present. - ai-review.ejs: file input (multiple, capture=environment) with client-side canvas downscaling (longest edge 1600px, JPEG q0.8) before upload, thumbnail previews with a remove button, images included in the preview POST body. - locales/en.json, locales/fr.json: new ai.import_photos_* and ai.err_image_rejected keys.
… race - setDisabled() no longer disables the enable toggle when settings come from env: resolveCandidate() only force-enables when an env API key is present, so a key-less local-endpoint env config (the documented Ollama/LM Studio setup) could never actually be turned on by hand. - apiKeyEl now clears the pending-clear flag on input, so clicking Clear key and then pasting a new one no longer silently saves the clear sentinel instead of the pasted key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t-in claim - Barcode fallback: correct the description to match resolveBarcodeWithAi exactly — text digits in, a search query out (never an image, never an invented item shown directly), a confidence threshold discards weak guesses, and the actual result still comes from the module's provider. - Local endpoints: both Ollama and LM Studio URLs were missing the required /v1 suffix that core/ai/client.ts posts to, which 404s. - 'Off by default': the barcode fallback actually runs automatically on every failed scan once AI is enabled (itemRoutes.ts calls it with no per-scan opt-in) — correct that claim while keeping the accurate 'you trigger text/photo import yourself' framing, and add where the import card lives (admin panel, Imports from external services). - Minor a11y: add alt text to the AI-import photo-picker thumbnail. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…a director - core/types.ts: add PluginDefinition.barcodeSearchFallback, for a provider whose free-text search can match raw barcode digits directly (Discogs). plugins/music/index.ts opts in; dvd/games are unaffected. - core/routes/itemRoutes.ts: when neither UPCitemdb nor the AI fallback resolves a barcode's title, a plugin with barcodeSearchFallback now searches with the raw scanned digits instead of dead-ending - this restores music's pre-supportsBarcodeSearch behavior (Discogs indexes barcodes), which regressed for AI-disabled installs when Task 8 turned supportsBarcodeSearch on for music. dvd/games keep the existing dead-end (TMDB/IGDB don't index raw digits). - plugins/dvds/index.ts: findDuplicate, findPotentialDuplicates and getVariants no longer build an unconditional director: /^$/i regex clause, which could only ever match another director-less item. An empty director (an AI-imported movie with nothing to go on, now legal since fff1254) now omits the director clause instead of guaranteeing a duplicate/variant miss against the real, enriched item. - core/routes/csvImportRoute.ts: apply the same plugin.includeCreatorInSearch conditional searchQuery that aiImportRoute.ts already used, so music/books enrich the same way regardless of which import path (CSV vs AI) was used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- sanitizePluginDraft now drops custom fields whose slugified name collides with RESERVED_FIELD_NAMES, matching buildConfigFromSubmission's validation. Without this, an AI-suggested field like "Description" passed the review form but only failed at Save time. - generatePluginDraft no longer swallows aiChat failures into a plain null return. A transport/provider error (bad key, wrong model, unreachable endpoint) now propagates so the route can surface the real message via the new ai_err_generation_failed i18n key, instead of always showing the generic "try rephrasing" copy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…generate spinner - Removed the redundant MAX_FIELDS/MAX_FORMATS slice ahead of the draft-cap slice in pluginGenerate.ts's fields/formats loops (already fixed by the smaller MAX_DRAFT_* cap). - create-plugin.ejs's description input now uses maxAiDescriptionChars from the route instead of a hardcoded 2000, so it can't drift from MAX_DESCRIPTION_CHARS. - The Generate button now swaps its icon to a spinner while a request is in flight, matching the pattern already used in admin.ejs and admin-instance.ejs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…pstream/main A conflict resolution while replaying these commits onto upstream/main left two lines (searchQuery/resolvedTitle assignment) outside the if/else that should have contained them, which would have let the barcodeSearchFallback path fall through and overwrite searchQuery with a still-null query. Caught by make typecheck (strict null check), fixed before push.
Owner
|
really need some time to review this but thanks a lot! pretty sure that is a big step for DVinyl |
Siebe-Uy
added a commit
to Siebe-Uy/DVinyl
that referenced
this pull request
Sep 3, 2026
…#138) can build on it # Conflicts: # docs/api-keys.md
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.
Summary
Adds an optional, bring-your-own-key AI assist to DVinyl, off by default and fully backward compatible.
AI assist (core):
AI plugin drafting:
/create-plugin), describe a collection type in a sentence (e.g. "old glass Coca-Cola bottles") and get the whole builder form pre-filled — name, icon, color, creator-field label, feature toggles, a handful of custom fields and formats — for review and editing with the existing live preview, before saving through the existing, unmodified save/validation path.Design notes
Testing