fix: stop gating Quickstart's OCR model dropdown on the vision heuristic - #240
Closed
admonstrator wants to merge 1 commit into
Closed
fix: stop gating Quickstart's OCR model dropdown on the vision heuristic#240admonstrator wants to merge 1 commit into
admonstrator wants to merge 1 commit into
Conversation
Background: Quickstart auto-detect classifies models by name heuristics only (services/quickstartService.js classifyModelName()), and the "Suggested OCR model" dropdown/checkbox in Step 5 was gated entirely on the resulting `visionModels` list. A dedicated OCR model with an unfamiliar name - e.g. Mistral's `mistral-ocr-latest` - matches none of the hardcoded vision hints (llava, pixtral, vision, gemma3, ...), so it classifies as `['text']` and never appears in that list. The dropdown showed "No vision-capable models found" and the "enable OCR" checkbox was disabled, even though the model exists and works - forcing users into manual configuration for any provider whose naming doesn't match the hint list. detectAndClassify() also always returned `ocrProvider: 'custom'`, never the `'mistral'` OCR provider that manual setup already fully supports (services/setupService.js validateOcrConfig, and the "Mistral OCR API" option in the always-visible "OCR fallback" wizard step). We considered parsing the `capabilities` object some OpenAI-compatible `/v1/models` responses include (Mistral does; LM Studio, Ollama, and plain OpenAI-compatible servers don't) to classify models more accurately, but rejected it: that's an undocumented, vendor-specific extension, not part of the OpenAI /v1/models spec, and would only special-case one provider while leaving every other naming scheme on the same guesswork - the actual underlying problem. Changes: - services/quickstartService.js: added `resolveOcrProviderDefault(url)`, a pure host-string check (mirrors the existing api.mistral.ai check in setupService.getMistralUrlValidationOptions) used in detectAndClassify() to default `ocrProvider` to `'mistral'` when the detected host is api.mistral.ai, `'custom'` otherwise. No classification logic changed - classifyModelName/classifyLmStudioEntry/classifyOllamaShowPayload are untouched. - public/js/setup.js runQuickstartDetect(): the OCR dropdown and "enable OCR" checkbox are now driven by the same non-embedding candidate list as the AI dropdown (`textModels` - every model is already classified as exactly one of ['embedding'] / ['text'] / ['text','vision'], so "has text capability" already means "not embedding-only"), instead of the vision-heuristic-filtered `visionModels`. A heuristic `suggestedOcrModel` is still pre-selected when available; when it isn't, the user picks from the full list themselves rather than seeing an empty/disabled dropdown. - public/js/setup.js applyQuickstartToManualFields(): uses `detection.ocrProvider` (the host-based default above) instead of a hardcoded `'custom'` literal. - views/setup.ejs: updated the dropdown label from "Suggested OCR model (vision-capable)" to "OCR model", since it's no longer filtered to heuristically vision-classified models. - A wrong provider default is never a dead end: the "OCR fallback" wizard step (its own always-visible step, not nested under the Quickstart/Manual AI toggle) lets the user change the OCR provider dropdown themselves before finishing setup regardless. - Cleaned up the same three pre-existing ESLint issues in setup.js as PR #238 (missing `/* global Swal */`, unused `catch (_error)` binding, stale `no-await-in-loop` disable comment) so the full file passes this repo's changed-files ESLint gate. Testing: - tests/test-quickstart-model-classification.js: added classifyModelName('mistral-ocr-latest') === ['text'] (documents why the OCR dropdown must not gate on the vision heuristic) and resolveOcrProviderDefault() cases for a Mistral host, a versioned Mistral URL, a local/non-Mistral host, and a blank host. - tests/test-setup-wizard-quickstart.js: added a second detection fixture (openai-compatible, api.mistral.ai, a model present only in textModels) asserting the OCR dropdown/checkbox stay enabled, manually selecting the dedicated OCR model flows through to the manual OCR model field, and the OCR provider defaults to 'mistral'. Verified this new assertion actually fails without the fix (reverted the source changes locally, confirmed the test catches the regression, then restored). - node scripts/run-tests.js --all: 42 passed, 7 skipped (server-dependent), 0 failed. - npx eslint clean on all changed JS files. - node scripts/regen-openapi.js produces no diff (no API surface change - the quickstart/detect response gains no new field, `ocrProvider`'s value just changes). Note: as in PR #238/#239, npx prettier --check still fails on setup.js/quickstartService.js/the two test files - all pre-existing formatting drift confirmed present on origin/main before this change, intentionally left untouched per the same discussion with the repo maintainer. Impact: Any OpenAI-compatible provider whose model names don't match the hardcoded vision hints (Mistral's OCR models, and any future differently-named dedicated OCR/vision model from any provider) can now be selected as the OCR model through Quickstart instead of forcing manual configuration. Detecting Mistral's own API additionally pre-selects the dedicated Mistral OCR provider path instead of the generic chat-completions path, which doesn't work for Mistral's non-chat OCR models (`completion_chat: false`). Upstream Status: new fix, not yet upstreamed to clusterzx/paperless-ai. Closes #236
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
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
classifyModelName()), and the "Suggested OCR model" dropdown/checkbox in Step 5 was gated entirely on the resultingvisionModelslist. A dedicated OCR model with an unfamiliar name — e.g. Mistral'smistral-ocr-latest— matches none of the hardcoded vision hints, so it classifies as['text']and never appeared in that list. The dropdown showed "No vision-capable models found" and the "enable OCR" checkbox was disabled, even though the model exists and works.detectAndClassify()also always returnedocrProvider: 'custom', never the'mistral'OCR provider that manual setup already fully supports.capabilitiesobject some OpenAI-compatible/v1/modelsresponses include (Mistral does this; LM Studio/Ollama/plain OpenAI-compatible servers don't). That's an undocumented, vendor-specific extension, not part of the OpenAI spec, and would only special-case one provider while leaving every other naming scheme on the same guesswork.textModels— every model is already exactly one of['embedding']/['text']/['text','vision'], so "has text capability" already means "not embedding-only"). A heuristicsuggestedOcrModelstill pre-selects a sensible default when one exists; otherwise the user picks manually instead of hitting a dead-end dropdown.resolveOcrProviderDefault()— a plain host-string check (mirrors the existingapi.mistral.aicheck insetupService.getMistralUrlValidationOptions) — so the OCR provider defaults to'mistral'only when the detected host is actually Mistral's API,'custom'otherwise. A wrong default is never a dead end: the "OCR fallback" step is its own always-visible wizard step (not nested under the AI step's Quickstart/Manual toggle), so the user can change the provider dropdown themselves regardless.views/setup.ejsfrom "Suggested OCR model (vision-capable)" to "OCR model" since it's no longer heuristic-filtered.setup.jsas fix: reset hidden aiProvider when switching to manual AI configuration #238 (missing/* global Swal */, unusedcatch (_error), staleno-await-in-loopdisable comment).Test plan
tests/test-quickstart-model-classification.js: addedclassifyModelName('mistral-ocr-latest') === ['text'](documents why the dropdown must not gate on the vision heuristic) andresolveOcrProviderDefault()cases (Mistral host, versioned Mistral URL, local/non-Mistral host, blank host).tests/test-setup-wizard-quickstart.js: added a second detection fixture (openai-compatible,api.mistral.ai, a model present only intextModels) asserting the OCR dropdown/checkbox stay enabled, manual selection flows through to the manual OCR model field, and the OCR provider defaults tomistral. Verified this assertion actually fails without the fix (reverted the source changes locally, confirmed red, restored).node scripts/run-tests.js --all: 42 passed, 7 skipped (server-dependent), 0 failed.npx eslintclean on all changed JS files.node scripts/regen-openapi.jsproduces no diff (no new response field, just a differentocrProvidervalue).npx prettier --checkstill fails onsetup.js/quickstartService.js/the two test files — pre-existing formatting drift confirmed present onorigin/mainbefore this change, intentionally left untouched per the same discussion with the repo maintainer.Closes #236
Generated by Claude Code