Skip to content

feat(recognition): add a shelf-scanner bench and retune the Qwen adapter - #43

Open
arenier wants to merge 9 commits into
mainfrom
claude/issue-10-vgaszo
Open

feat(recognition): add a shelf-scanner bench and retune the Qwen adapter#43
arenier wants to merge 9 commits into
mainfrom
claude/issue-10-vgaszo

Conversation

@arenier

@arenier arenier commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Livre l'étape 6 de #10 — le bench qualité qui départage les deux adapters ShelfScannerPort
sur photos réelles — et retouche l'adapter Qwen + le prompt de scan à la lumière de ce que le
bench a révélé (les étapes 1-5 et 7 sont déjà sur main, PR #39). C'est l'instrumentation de
phase 3 prévue par l'ADR 0005.

Contexte

Le contexte recognition a ses deux adapters (Gemini, Qwen/OpenRouter) derrière le port, l'endpoint
POST /scan et les tests sur réponses enregistrées. Ce qui manquait : de quoi choisir lequel des
deux devient le défaut
, mesuré sur de vraies étagères plutôt qu'au jugé. Le bench a servi tout de
suite : il a montré l'adapter Qwen instable et le prompt trop timide, d'où leurs retouches ici.

Modifications

Regroupées par intention :

Projet / lib Ce que ça établit
libs/shared/text-match Primitive partagée : normalisation + égalité floue (Levenshtein, seuil 0.85). Sert au scoring et à la déduplication du bench ; la réconciliation s'en resservira.
tools/bench (logique pure) Vérité terrain YAML (zod), scoring (rappel, précision, exactitude par champ, permutations, hallucination) avec déduplication des détections répétées, coût/usage, sélection de fournisseur, rendu Markdown. Testé en CI, sans réseau.
tools/bench (runner live) Instrumente fetch (latence, tokens), appelle les deux adapters, écrit le tableau. Jamais en CI — un appel payant par photo.
libs/recognition/infrastructure (prod) Adapter Qwen : défaut qwen3-vl-235bqwen2.5-vl-72b ; json_objectjson_schema ; max_tokens. Et le prompt de scan partagé rendu exhaustif (balayage systématique de toute l'étagère) — le modèle s'arrêtait trop tôt. Le prompt interdit aussi les champs vides (le domaine rejette tout le payload sur un champ vide) et les doublons.
package.json (racine) tools/* ajouté au glob des workspaces Yarn. Raccourci yarn bench.
Docs Protocole (tools/bench/README.md), modèle de vérité terrain, note de décision docs/decisions/0001, carte d'architecture CLAUDE.md.

Le bench est un projet Nx dédié tagué type:app / scope:api. La logique pure reste sans provider.

Tests

yarn check (lint + test + build, 8 projets) vert. Notamment :

 recognition-infrastructure   30 tests   (Qwen : json_schema, max_tokens, modèle par défaut)
 bench                        39 tests   (scoring + déduplication, render, ground-truth, usage, config)

Le prompt est partagé par les deux adapters à dessein (le bench mesure les modèles, pas les
prompts) ; sa qualité se valide par le bench manuel, pas par un test unitaire (ADR 0005). Le runner
live n'a pas de test automatisé (un appel payant par photo) ; exécuté à la main sur le jeu de
référence — voir Points d'attention.

ADR concernés

  • ADR 0005 — VLM seul derrière le port ; le
    bench est son instrumentation de phase 3. Le choix de modèle, de format et le prompt sont des
    décisions de niveau inférieur, réversibles par configuration, hors ADR.
  • ADR 0002 — frontières hexagonales : seul un
    type:app connaît infrastructure, d'où le tag du bench.
  • ADR 0008 — catégories strictes respectées.

Aucune décision structurante sans ADR ; le rejet en bloc d'un payload à champ vide (ADR 0005) est
conservé — c'est le prompt qui empêche les champs vides, pas le validateur qui s'assouplit.

Points d'attention

  • Détection : nettement améliorée par le prompt exhaustif, pas encore mesurée. Sur un probe
    jetable (même modèle/prompt), Qwen 72b passe de 7 → 19 livres sur une étagère clairsemée et de
    10 → 42 sur une dense — le modèle s'arrêtait trop tôt, pas un problème d'image ni de tokens
    (complétion loin du plafond). Le prompt agressif rouvrait deux défauts, tous deux traités ici :
    champs vides (le prompt les interdit, sinon le domaine perd toute la photo) et doublons
    (dédupliqués dans le scoring). Reste à confirmer contre la vérité terrain.
  • La sélection du fournisseur n'est pas faite : elle exige une vérité terrain vérifiée à la
    main
    . Tant qu'elle n'est pas saisie, SHELF_SCANNER_PROVIDER reste à stub.
  • tools/bench/tsconfig.lib.json épingle lib: es2023 (pour toSorted) — outil Node-26 only.
  • La logique de scoring (matching glouton, permutations, hallucinations, déduplication) est à relire
    en premier : tools/bench/src/lib/scoring.ts et ses tests.

Hors scope

  • La sélection du gagnant et la bascule du défaut SHELF_SCANNER_PROVIDER (après vérité terrain).
  • Le découpage d'image en tuiles (option C, ADR 0005) — le levier suivant pour la détection.
  • Toute persistance des photos ou des résultats — la sortie du bench est gitignorée.

Avance #10 (étape 6) — ne clôt pas l'issue : la sélection du fournisseur et la bascule du défaut
restent en attente de la vérité terrain vérifiée à la main.

🤖 Generated with Claude Code

Step 6 of #10: a manual bench that pits the two ShelfScannerPort adapters
(Gemini, Qwen/OpenRouter) against each other on real shelf photos, to pick the
production default — the phase-3 instrumentation ADR 0005 calls for.

- libs/shared/text-match: reusable string normalization + fuzzy equality
  (Levenshtein ratio, threshold 0.85). A shared primitive: the bench scores a
  detection against the ground truth with it, and bibliographic reconciliation
  will match reads against the reference the same way.
- tools/bench: dedicated Nx project (type:app / scope:api, so it may know
  infrastructure and wire the real adapters). Pure, CI-tested logic — ground-truth
  YAML loading (zod), scoring (recall, precision, per-field accuracy, structuring
  errors, high-confidence hallucination), Markdown rendering — and a live runner
  that instruments fetch for latency and token usage. The runner is never in CI:
  it costs a paid call per photo.
- Add tools/* to the Yarn workspaces so the project is linked and keeps its Nx
  tags (hence the module boundaries).
- Docs: bench protocol (tools/bench/README.md), ground-truth template, and a
  lower-level decision note (docs/decisions/0001), updated from a live run.

Live run (2026-09-02): Gemini reads end to end (9/10 photos, one transient 503,
~0.27c/scan, ~20s median). Qwen is unreachable from this environment — its
network policy blocks openrouter.ai (403 at the proxy); the adapter correctly
surfaces that as ShelfScanFailed. Quality metrics (recall/precision/hallucination)
stay pending: they need a human-verified ground truth, which by design a VLM
draft cannot stand in for.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k
OpenRouter is reachable now that its domain is on the environment egress. Both
providers ran end to end: near-equal cost (~0.25c vs ~0.23c/scan), Qwen ~7x
faster, but Qwen is unstable under the shared json_object prompt — 5 of 10 photos
return 0 books, one returns 158, one fails on truncated JSON — while Gemini stays
regular via native schema-constrained decoding. An operational signal, not the
quality verdict: that still waits on the human-verified ground truth.

Also document that Node's fetch needs NODE_USE_ENV_PROXY=1 to honor HTTPS_PROXY
in a proxied environment, otherwise calls 403 on the egress allowlist.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k
Wraps `nx build bench && node tools/bench/dist/main.js` behind `yarn bench`,
documented in tools/bench/README.md and the CLAUDE.md command list. Env vars go
in front, e.g. `BENCH_PROVIDERS=qwen yarn bench`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k
…SON (#10)

The Qwen adapter asked for `response_format: json_object` — free-form JSON. On
dense shelves that let the model truncate its answer mid-structure and emit empty
author/title fields, failing whole scans (bench run on #10: 0 books on half the
photos for the 235B, one 158-book reply, truncated JSON). Switch to
`response_format: json_schema` carrying `SHELF_SCAN_JSON_SCHEMA` — the same
contract Gemini already decodes against, now held over Qwen's grammar. All three
candidate Qwen-VL models advertise structured-output support on OpenRouter.

`strict` narrows the shape, not the meaning: the answer is still validated
downstream by the shared response mapper, so an off-contract value still fails
closed rather than leaking a bad DetectedBook.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k
…tokens (#10)

The bench default 235B was unusable on dense shelves — 0 books on half the
photos, truncated JSON on others — and json_schema alone did not rescue it. The
72B (the OCR-focused VL line) stays stable, so it becomes the Qwen default in the
adapter and the bench. Also send a generous max_tokens so a legitimate long list
is not cut off mid-JSON; a runaway repetition still hits the cap and fails, which
is the outcome we want.

This does not fix the deeper issue that all providers under-detect on these dense
ressourcerie shelves — that is the finding logged on #10, and needs the
human-verified ground truth to quantify.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k

arenier commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

Review · 🔴 À ne pas merger

Réserves — Lint max-lines introduit par le diff, CI rouge (libs/recognition/infrastructure/src/lib/qwen-shelf-scanner.adapter.ts:73) · Logique pure coût/usage non testée (tools/bench/vitest.config.mts:12)

La PR livre l'étape 6 de #10 : le bench qualité qui départage les deux adapters ShelfScannerPort sur photos réelles, plus la lib partagée shared-text-match. Le diff est soigné et bien documenté, et les frontières Nx sont correctes. Mais il embarque aussi un changement du vrai adapter Qwen (modèle par défaut + décodage contraint par schéma) dont les ajouts font franchir la limite max-lines-per-function d'oxlint à deux fonctions — ce qui fait échouer la CI (check). Verdict 🔴 par le code, pas par la CI.

CI rouge sur check (oxlint) — la cause est un défaut du diff, reprise ci-dessous en bloquant.

🔴 Bloquants

  • Lint max-lines-per-function introduit par le diff (libs/recognition/infrastructure/src/lib/qwen-shelf-scanner.adapter.ts:73, libs/recognition/infrastructure/src/lib/qwen-shelf-scanner.adapter.spec.ts:94) — l'ajout du bloc response_format: { type: 'json_schema', … } + max_tokens fait passer la méthode privée post à 51 lignes (max 50) ; les nouveaux it poussent une fonction du spec à 61 lignes. yarn oxlint --type-aware s'arrête sur ces deux erreurs (catégories strictes, ADR 0008) → CI check en échec. La description annonce pourtant « yarn check … vert » et « fonctions courtes » : ce n'est plus vrai sur la tête. → Extraire un helper (p. ex. buildRequestBody/buildResponseFormat) pour ramener post sous 50 lignes, et factoriser/scinder le bloc de test concerné.

🟠 À corriger

  • Logique pure de usage.ts et config.ts hors périmètre de test (tools/bench/vitest.config.mts:12) — l'include est limité à src/lib/**, ce qui exclut des fonctions pures et sans réseau : estimateCost/readUsage (usage.ts) et mediaTypeOf/providerSpec (config.ts, provider inconnu → throw, formule de coût, parsing des deux schémas d'usage). Ces chiffres alimentent directement la note docs/decisions/0001 qui posera le défaut de prod ; une régression y passerait inaperçue. Le commentaire du vitest.config.mts ne justifie que l'exclusion du runner live (appel payant), pas celle de ces fonctions, testables comme scoring/render/ground-truth. → Déplacer ces fonctions vers src/lib/ (ou élargir l'include à ces fichiers) et ajouter les specs (mapping d'extension, provider inconnu, coût avec/sans reportedCostUsd, les deux formes de payload usage).

💬 Questions

  • tools/bench/tsconfig.lib.json:9 (lib: es2023) — déviation du repo (es2022), assumée dans la description et justifiée par toSorted() sur un outil Node-26-only jamais bundlé ailleurs. Pas d'objection.
  • Le titre feat(bench): (qui deviendra le sujet du squash sur main) sous-vend le changement de comportement de prod de QwenShelfScannerAdapter (modèle par défaut qwen3-vl-235bqwen2.5-vl-72b, json_objectjson_schema, max_tokens). Ce changement est bien testé (nouveaux it sur réponses enregistrées, ADR 0005) — la remarque porte sur la trace laissée sur main, pas sur le fond.

✍️ Style & altitude

Rien à signaler. Les commentaires portent systématiquement un pourquoi (glouton plutôt qu'assignation optimale, zéro plutôt que NaN, séparation logique pure / réseau, choix du 72B) et non un quoi — exactement ce que demande CLAUDE.md.

Relecture statique : lecture de code + CI (read-only). Ni lint, ni test, ni build lancés — un 🟢 veut dire « rien trouvé en lecture », pas « ça compile ».


Generated by Claude Code

Repository owner deleted a comment from claude Bot Sep 2, 2026
@arenier
arenier marked this pull request as draft September 2, 2026 23:00
… pure logic

Review triage on #43.

- max-lines-per-function (blocking, CI red): the json_schema + max_tokens block
  pushed QwenShelfScannerAdapter.post past 50 lines and a spec describe past 50.
  Extract requestBody() from post, split the request-shape describe. oxlint green.
- Untested pure logic: the bench vitest include was src/lib/** only, leaving the
  cost/usage parsing (usage.ts) and provider selection (config.ts) — which feed the
  decision-note numbers — outside CI. Widen the include to src/**, export readUsage,
  and add specs for estimateCost, readUsage (both payload shapes), mediaTypeOf,
  env/requireEnv and selectedProviders (unknown provider throws). The live runner
  still has no spec, so nothing paid runs in CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k
@arenier arenier changed the title feat(bench): add a VLM provider bench for the shelf scanner feat(recognition): add a shelf-scanner bench and retune the Qwen adapter Sep 2, 2026

arenier commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

Triage de la review du 02/09. Les deux points ont été vérifiés dans le code (le 🔴 reproduit par oxlint --type-aware, le 🟠 par lecture de l'include vitest) : les deux sont fondés. Corrigés dans e353967, yarn check local repassé au vert. PR remise en draft le temps du tour.

Point Verdict Traitement
🔴 max-lines-per-function (post 51 · spec 61) → CI check rouge Fondé ✅ Corrigé e353967
🟠 Logique pure usage.ts/config.ts hors périmètre de test Fondé ✅ Corrigé e353967
💬 lib: es2023 (déviation es2022) Constat (sans objection du relecteur) ⛔ Rien à faire
💬 Titre feat(bench): sous-vend le changement de prod Qwen Fondé ✅ Corrigé (retitré + description rafraîchie)
Vérifications point par point (2)

1. max-lines-per-function — reproduit, puis corrigé

Reproduit sur la tête avant correctif (la cause exacte invoquée, pas la CI seule) :

$ yarn oxlint --type-aware
qwen-shelf-scanner.adapter.ts:73:21  error  max-lines-per-function: `post` has too many lines (51). Max 50.
qwen-shelf-scanner.adapter.spec.ts:94:56  error  max-lines-per-function: function has too many lines (61). Max 50.
exit 1

CI check = failure confirmée côté GitHub. Ma revendication « yarn check vert » n'était donc plus vraie sur la tête — j'avais lancé nx lint (ESLint/frontières) mais pas oxlint --type-aware (qui porte max-lines) après les commits json_schema/max_tokens.

Correctif : post() délègue la construction du corps à une méthode privée requestBody() ; le describe du spec est scindé en deux. Après :

$ yarn oxlint --type-aware   → exit 0
$ yarn check                 → exit 0  (lint + test + build, 8 projets)

2. Logique pure non testée — fondé, comblé

L'include vitest du bench était src/lib/**, ce qui laissait hors CI des fonctions pures qui alimentent les chiffres de docs/decisions/0001 : estimateCost/readUsage (usage.ts), mediaTypeOf/env/requireEnv/selectedProviders (config.ts).

Correctif : include élargi à src/** (un spec est découvert par ce glob, jamais un module source seul — le runner live n'a toujours aucun spec, donc rien de payant en CI) ; readUsage exporté ; deux specs ajoutés :

  • usage.spec.tsestimateCost (coût rapporté / estimé au token / provider inconnu → null / rien consommé → null) et readUsage (les deux formes de payload usage + corps non-JSON).
  • config.spec.tsmediaTypeOf (extensions connues/inconnues), env/requireEnv (trim, absent → throw), selectedProviders (deux défauts + modèles épinglés, override modèle, provider inconnu → throw).

Bench : 22 → 37 tests.

Point non relevé · Ce qui reste ouvert

Point non relevé par la review — la description de la PR était périmée : elle affirmait « Qwen n'a pas pu être mesuré » et le mettait hors scope, alors que depuis, OpenRouter a été rendu joignable, les deux fournisseurs ont été mesurés, et l'adapter Qwen a changé de défaut (235b72b), de format (json_schema) et gagné un max_tokens. Corrigé : titre → feat(recognition): add a shelf-scanner bench and retune the Qwen adapter, description réécrite.

Ce qui reste ouvert (📌, non traité ici) :

  • Détection insuffisante : même Gemini sous-liste ces étagères denses ; le json_schema n'a pas rescapé le 235B. C'est le vrai chantier ouvert par le bench — hors scope de ce triage.
  • Sélection du gagnant + bascule du défaut SHELF_SCANNER_PROVIDER : en attente de la vérité terrain vérifiée à la main.

Generated by Claude Code

Follow-up on the bench findings (#10).

- Prompt: the shared scan prompt now pushes exhaustiveness (scan the whole shelf
  band by band, a short list means stopping too early). On a throwaway probe this
  roughly tripled Qwen's reads on a sparse shelf (7 -> 19) and quadrupled them on
  a dense one (10 -> 42). To keep that recall without breaking the contract, it
  also forbids empty author/title (the domain rejects the whole payload on an
  empty field) and asks for each book once.
- Scoring: dedupe detections on the normalized (author, title) before matching, so
  a spine the model lists twice is not double-counted nor charged as a false
  positive. Keyed on shared-text-match normalization; the highest-confidence copy
  survives.

The prompt is shared by both adapters on purpose (the bench measures models, not
prompts); its quality is validated by the manual bench, not a unit test (ADR 0005).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k
Rename the 10 reference-shelf photos from their camera basenames
(20260801_HHMMSS.jpg) to shelf-fixture-1..10.jpg, on the GCS bucket and in
every tracked reference: the ground-truth template, the config spec example,
and the Gemini recorded-response provenance note. The bench discovers photos
dynamically, so no runner code changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k
shelf-fixture-2 framed the same shelf as shelf-fixture-1, only less wide, so
it added no coverage to the reference set. Removed from the GCS bucket and from
the ground-truth template. Remaining fixtures keep their numbers (1, 3..10) so
existing identifiers stay stable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGqVYV5GLC8qBTSKqAq19k
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants