Skip to content

Revue de code globale — fin Phase 3 (2026-05-14)

Scope : audit de la Phase 3 (entre v0.4.0 et HEAD), 30 commits, 111 fichiers modifiés (+13 373 / −433 lignes). 4 livraisons fonctionnelles : (1) foundation prompt management + scoring (livré 2026-05-10, 6 sous-PRs PR1→PR6 — Flyway V8 prompt_template + prompt_score, TickerNarrativePromptService, PromptScoreRecorder, page /settings/prompts avec éditeur + diff + activate, page stats par prompt, thumbs feedback PATCH idempotent) ; (2) page observabilité narrative (livré 2026-05-13, 3 sous-PRs PR1→PR3 — NarrativeObservabilityQuery + Service + Controller, page Angular per-symbol + index, filtres date + prompt + thumbs client-side, lien depuis le dossier ticker) ; (3) score de cohérence cross-runs (livré 2026-05-14 — CoherenceScorer pure-function, chip OK/WARN/HIGH sur chaque card de la timeline avec tooltip 5 lignes) ; (4) détection de biais (livré 2026-05-14 — NarrativeBiasQuery + Service + DTOs, page /observability/bias avec 4 sections : sentiment distribution, calibration sentiment vs prix, topic coverage, thumbs distribution). Côté navigation : entrée navbar « Observabilité » ajoutée après Dashboard, back-link de la timeline per-symbol redirige vers l'index. Patches de maintenance hors features : bumps deps Angular 21 (commits 09ab31e, 898dcce), ticker.scss budget bump 24→32 kB (ad792c4), NG0955 keying fix (410da26).

Méthode : 3 subagents general-purpose Claude lancés en parallèle sur des slices cohérentes (foundation prompt management 2026-05-10 ; page observabilité 2026-05-13 ; cohérence + biais 2026-05-14), chacun briefé sur les conventions ground truth (CLAUDE.md, architecture.md, ddd.md) et sur l'audit précédent. Sortie : punch-lists structurées (Bloquants / À discuter / Mineurs) avec fichier:ligne + suggestion + rationale. Vérification croisée des « bloquants » par le main thread avant remontée — un faux positif filtré (cf. infra). Les findings non bloquants ont été soit patchés, soit filed en dette technique (cf. backlog.md > Dette technique > Coutures post-livraison Phase 3 — résidus review 2026-05-14 + ticket 🟡 dédié Bar-lookup priceAtOrAfter).

État du commit au moment de la revue : branche master, dernier commit committé 5fa37fe feat(observability): bias dashboard + navbar entry + back-link to index. Working tree dirty avec le patch issu de cet audit (3 fichiers : TickerNarrativePromptService.kt + son test + backlog.md). 4 livraisons Phase 3 toutes mergées. 14 repositories frontend (10 Phase 2 + 4 Phase 3 : Prompt, NarrativeFeedback, NarrativeObservability, NarrativeBias). Migrations Flyway V1→V8. Tests : ~2 200 lignes Kotlin + ~1 800 lignes TS ajoutées sur la phase. Tous les findings critiques de l'audit 2026-05-10-fin-phase-2.5.md ont été clôturés avant v0.4.0 (cf. journal-livraisons.md > Phase 2.5 > retour audit fin Phase 2.5 (4 findings clôturés)) — pas de carry-over.

Résumé exécutif

App globalement saine pour un tag candidat v0.5.0. La Phase 3 a livré une boucle d'audit narrative complète en 5 jours (foundation prompt management → timeline observabilité → cohérence chip → biais dashboard) qui transforme le corpus accumulé depuis Phase 1 en signal exploitable. Les patterns établis ont tenu : ports + adapters frontend cohérents (4 nouveaux repositories suivent le <name>.repository.ts + adapters/<name>.http.ts à la lettre), services Spring proprement isolés en application/ avec persistence en infrastructure/persistence/, native SQL réservé aux cas où JPQL devient maladroit (LATERAL joins sur prompt_score, percentiles + date_trunc sur les stats prompt, agrégats GROUP BY sentiment sur les biais). Les nouveaux pure-function helpers (CoherenceScorer, NarrativeBiasService.tokenize) sont déterministes, sans dépendance Spring, fully unit-tested. La discipline tests-as-documentation reste très bonne : noms de tests = phrases anglaises de spec, fixtures factory + override par scénario, comments expliquent le why des edge cases.

Risque principal identifié — patché en session avant cet audit : TickerNarrativePromptService.lookupOrFallback mettait la FALLBACK_TEMPLATE synthétique dans le cache Caffeine au même titre qu'une vraie row. Conséquence : un boot pendant que Flyway V8 applique (cas typique d'un fresh deploy), ou un seed BDD manuellement wipé, causait une fenêtre de 60 s pendant laquelle chaque narratif ignorait le prompt seedé en BDD ET trippait isFallback → PromptScoreRecorder skippait son write → snapshot persisté avec prompt_template_id = null. Single-user local masquait largement le sujet (V8 a tourné depuis longtemps), mais la classe de bug est sérieuse pour Phase 5 deploy. Patch posé : cache.getIfPresent(name)?.let { return it } puis lookup direct, cache.put(name, row) uniquement sur succès BDD ; le fallback short-circuite le caching. Test pin de régression fallback is never cached so a transient empty-DB state self-heals on the next request ajouté dans TickerNarrativePromptServiceTest.

Risque important non patché — filed dette tech 🟡 : NarrativeObservabilityService.priceAtOrAfter (et son jumeau dans NarrativeBiasService) ne guarde pas la borne basse. Si un snapshot est généré avant le 1er bar du chart 1Y (un narratif d'avril 2025 visualisé en mai 2026), target.plusDays(1) reste antérieur au 1er bar disponible, donc on retourne le 1er bar du chart — soit ~12 mois après le snapshot — et on présente cette différence comme « delta1d ». Affichage trompeur. Devient observable une fois le corpus à 1+ an d'histoire (donc pas avant ~fin 2026), pas urgent mais à patcher avant de consulter sérieusement la timeline historique sur des snapshots vieux.

Faux positif filtré : un subagent a flaggé en bloquant un test failure putatif sur CoherenceScorer.priceMove (claim que BigDecimal.ZERO scale 0 ≠ bd("0.0000") scale 4). Vérification main thread : priceMove n'a pas de fast-path zero — current.subtract(previous).divide(previous, 4, HALF_UP) est inconditionnel et préserve la scale. subtract retourne 0.0000 scale 4, divide retourne 0.0000 scale 4, assertEquals passe. L'agent a halluciné un fast-path qui n'existe pas. Pas de patch.

Forces : (a) 3 reviews subagent en parallèle au lieu d'un seul a fonctionné — chaque agent reste dans une fenêtre de contexte gérable, les 3 vues recoupent les fichiers communs (NarrativeObservabilityService modifié à la fois par #2 et par les wires #2/#3) sans rater de drift. Le main thread synthétise et arbitre. (b) Tests-as-documentation très bien tenus sur les 4 livraisons : CoherenceScorerTest (9 tests dont les seuils 60 % bias, FLIPPED + 5 % move = OK, jaccard normalise case+trim) ; NarrativeBiasServiceTest (10 tests dont zero-pad 3 buckets, calibration skip null deltas, one-fetch-per-symbol, MarketUnavailableException per-symbol survive) ; TickerNarrativePromptServiceTest (cache-miss self-heal post-patch). (c) Routing precedence double-testée sur les segments littéraux /tickers, /bias qui doivent être déclarés avant /{symbol} — backend (@WebMvcTest) ET frontend (app.routes.ts test implicite via les specs page). (d) i18n parité FR/EN maintenue sur l'ensemble des nouvelles clés (prompts.*, prompt-stats.*, observabilityPage.*, observabilityIndexPage.*, biasPage.*, nav.observability). (e) Décision « pas de LLM-as-judge » documentée et défendue sur les 2 scoring (cohérence + biais) — heuristique pure pour rester gratuit, déterministe, transparent ; LLM-as-judge filed pour v2 si signal heuristique insuffisant.

Critique

(blocker ou risque sévère — corruption data, sécurité, contrat cassé)

0. (Patché en session) TickerNarrativePromptService.lookupOrFallback cachait le FALLBACK_TEMPLATE synthétique

  • Fichier : backend/src/main/kotlin/com/portfolioai/analysis/application/TickerNarrativePromptService.kt:187-201 (avant patch).
  • Symptôme : cache.get(name) { repository.findFirstByNameAndIsActiveTrue(name) ?: FALLBACK_TEMPLATE } met indistinctement la fallback dans Caffeine. Si le 1er appel post-boot tombe pendant l'application de Flyway V8 (cas typique d'un fresh deploy, ou d'un wipe seed manuel), la fallback est cachée pour le TTL (1 min). Pendant cette fenêtre : (a) chaque narratif ignore le prompt seedé en BDD même après que V8 a fini, (b) isFallback(template) retourne true → PromptScoreRecorder skippe son write côté TickerNarrativeExecutor → snapshot persisté avec prompt_template_id = NULL (FK SET NULL respecté, pas de crash, mais corrupt observability).
  • Impact : observabilité narrative trouée pour la fenêtre de 60 s × tous les runs concurrents. Single-user local masque largement (V8 a tourné depuis longtemps sur tous les checkouts existants), mais la classe de bug est sérieuse pour Phase 5 deploy + tout futur scenario où un seed est wipé/reset.
  • Patch posé : lookup direct, cache.put(name, row) uniquement sur succès BDD. Fallback short-circuite le caching : 
    cache.getIfPresent(name)?.let { return it }
val row = repository.findFirstByNameAndIsActiveTrue(name)
if (row != null) { cache.put(name, row); return row }
log.warn(...)
return FALLBACK_TEMPLATE
  • Test de régression : TickerNarrativePromptServiceTest.fallback is never cached so a transient empty-DB state self-heals on the next request séquence Mockito .willReturn(null).willReturn(dbActiveRow()) puis verify(repository, times(2)).findFirstByNameAndIsActiveTrue(...) — pin que la 2ᵉ lecture re-hit la BDD au lieu de servir une fallback cachée.

Important

(à traiter en priorité)

1. priceAtOrAfter ne guarde pas la borne basse — deltas absurdes sur snapshots > 1Y

  • Fichiers : backend/.../NarrativeObservabilityService.kt:193-196 + NarrativeBiasService.kt:255-258 (helper privé dupliqué — candidat à extraction commune en passant).
  • Symptôme : bars.firstOrNull { it.timestamp >= target } couvre la borne haute (target au-delà de la dernière bar → null retourné, delta affiché « — »). Pas de symétrique pour la borne basse : si target = generatedDate.plusDays(1) est antérieur au 1er bar du chart 1Y (cas d'un snapshot daté d'avant la fenêtre 1Y, donc d'un narratif vieux de >1 an consulté aujourd'hui), la 1ʳᵉ bar du chart matche. On retourne donc une bar vieille de ~12 mois, et le delta calculé (bar1.close - snapshot.price) / snapshot.price est présenté comme « delta1d » dans la card timeline ou dans la moyenne de calibration.
  • Impact : (a) timeline observabilité : un user qui scroll vers le bas sur un ticker actif depuis 2024 verra des deltas faux sur les 12 derniers mois de la liste (sans signal visuel pour distinguer une donnée fiable d'un faux). (b) calibration biais : la moyenne avgDelta1d/1w/1m peut être biaisée par ces contributions trompeuses. Devient observable une fois le corpus à 1+ an d'histoire — donc pas avant ~fin 2026.
  • Fix : guard target >= bars.first().date côté priceAtOrAfter ; si en-dessous, retourner null (le delta apparaît « — » comme pour la borne haute).
  • Tests à ajouter : (1) NarrativeObservabilityServiceTest — snapshot daté avant la 1ʳᵉ bar du chart → tous les 3 deltas null. (2) NarrativeBiasServiceTest — calibration skip ce snapshot dans la moyenne (snapshotsWithDelta1d reste à N-1).
  • État : filed dette technique 🟡 « Bar-lookup priceAtOrAfter — guard la borne basse » dans backlog.md.

Modérée

(non bloquant, à patcher quand l'occasion se présente)

2. PromptScoreService.setThumbs race théorique sur deux PATCH concurrents

  • Fichier : backend/.../analysis/application/PromptScoreService.kt:55-99.
  • Symptôme : setThumbs fait un read-then-write sous @Transactional (default propagation). Deux PATCH concurrents sur le même snapshot_id peuvent tous les deux passer le findFirstBySnapshotId == null check et tous les deux insérer une row prompt_score. Pas d'unique constraint sur prompt_score(snapshot_id) côté schéma. Le PromptScoreRecorder (PR2) écrit aussi une row par run, donc la table peut contenir N rows par snapshot — la requête de lecture filtre par ORDER BY created_at DESC LIMIT 1 ce qui rend le sujet académique en pratique, mais l'asymétrie « write multiplie, read prend la dernière » pourrait surprendre une future feature qui agrège les scores.
  • Impact : nul en single-user (zéro PATCH concurrent en pratique). Worth a future V9 UNIQUE INDEX idx_prompt_score_snapshot_unique ON prompt_score(snapshot_id) WHERE snapshot_id IS NOT NULL pour blinder.
  • État : filed dette technique 🟢 dans le ticket umbrella « Coutures post-livraison Phase 3 ».

3. NarrativeBiasService.computeCalibration fan-out chart séquentiel

  • Fichier : backend/.../analysis/application/NarrativeBiasService.kt:117-124.
  • Symptôme : rawSnapshots.map { it.symbol }.distinct().associateWith { symbol -> chartClient.fetchChart(symbol, "1y", "1d").bars } enchaîne les fetchs en série. Pour 10 symbols sur cold cache Caffeine, ça fait 10× la latence single-call (typiquement 1-3 s × 10 ≈ 10-30 s). Le test calibration fetches the chart exactly once per unique symbol pin l'invariant « 1 fetch par symbol » mais pas la parallélisation.
  • Impact : acceptable v1 (single-user, ≤10 symbols typiques, souvent warm en cache via dossier ticker). Devient pertinent quand PortfolioAggregation Phase 4 consommera ce service depuis un thread request, ou quand la page bias sera ouverte plus souvent.
  • Fix futur : parallelStream Java ou runBlocking { withContext(Dispatchers.IO) { ... } } Kotlin coroutines. Bornage à 4 fetchs concurrents pour ne pas hammer Twelve Data.
  • État : filed dette technique 🟢 dans le ticket umbrella.

4. Bias flag rounding edge — pas de test sur cas fractionnaire

  • Fichier : backend/.../analysis/application/NarrativeBiasService.kt:94-100 + NarrativeBiasServiceTest.kt:80-95.
  • Symptôme : BIAS_THRESHOLD = BigDecimal("0.6000"), percent calculé via divide(scale=4, HALF_UP). Le test pin le seuil sur 60/100 = 0.6000 → flag fires (pass). Mais 5995/10000 = 0.5995 → 0.6000 après HALF_UP scale 4 → flag fires aussi. Pas de test pour documenter ce comportement de rounding au seuil. À l'inverse, 599/1000 = 0.5990 → no flag.
  • Impact : nul (le contrat est défendable — HALF_UP au scale 4 — mais non documenté). Risque qu'un futur dev change la RoundingMode ou la scale et casse silencieusement le seuil de bias flag.
  • Fix : 1 test ajoutant 5995/10000 → flag fires et 599/1000 → no flag, avec un commentaire qui pin la règle de rounding comme contrat.
  • État : filed dette technique 🟢 dans le ticket umbrella.

Mineur

(polish, refacto opportuniste)

5. V8 migration : commentaire claim de robustesse à corriger

  • Fichier : backend/src/main/resources/db/migration/V8__prompt_template.sql (commentaires de tête + autour du backfill).
  • Symptôme : la migration claim que le backfill UPDATE ticker_narrative_snapshot SET prompt_template_id = (SELECT id FROM seeded) WHERE prompt_version = 'v2' est sûr. En réalité un narratif persisté pendant l'exécution de la migration tomberait dans le gap et lariverait avec prompt_template_id = NULL malgré son prompt_version = 'v2'. Single-user ⇒ académique (Tilt restart séquentiel, pas de write concurrent), mais le commentaire affirme une garantie qu'il n'a pas.
  • Fix : soit dropper la claim et documenter explicitement la limitation single-user, soit ajouter LOCK TABLE ticker_narrative_snapshot IN ACCESS EXCLUSIVE MODE; au début de V8 (no-op single-user mais documente l'intention pour Phase 5 deploy).

6. NarrativeObservabilityQuery + NarrativeBiasQuery : LIMIT $constant interpolé au lieu de bound

  • Fichiers : NarrativeObservabilityQuery.kt:81 (LIMIT $MAX_ROWS) + :131 (LIMIT $MAX_TICKERS) + NarrativeBiasQuery.kt:118 (LIMIT $MAX_RAW_ROWS).
  • Symptôme : ces 3 LIMIT sont interpolés dans la string SQL via Kotlin string template, alors que le reste de la query bind tout via setParameter. Pas une faille (Int constants), mais incohérent et signale un drift.
  • Fix : setParameter("limit", MAX_ROWS) puis LIMIT :limit dans la SQL string.

7. Tokenizer biais — stopwords dupliqués + incohérence « price »

  • Fichier : backend/.../analysis/application/NarrativeBiasService.kt:288-379 (STOPWORDS).
  • Symptôme A : "his" apparaît 2× dans la liste, "side" aussi. setOf dedupe silencieusement donc cosmétique, mais signale que la liste n'a pas été review end-to-end.
  • Symptôme B : "price" est stopworded — incohérent avec le DTO comment TopicCoverageDto qui claim que « never said price » serait un signal pertinent (« the inverse read is « topics never mentioned » »). Trancher : strip OU surface, et aligner le commentaire.
  • Fix : dedupliquer + arbitrer sur "price". Idem audit léger sur le reste de la liste pour repérer d'autres faux strips.

8. bias.ts / observability.ts : buildFilter + nextDayIso dupliqués entre les 2 pages

  • Fichiers : frontend/src/app/features/observability/observability.ts:188-206 + frontend/src/app/features/observability/bias/bias.ts:113-131.
  • Symptôme : les 2 méthodes sont identiques au caractère près. Le commentaire dans bias.ts reconnaît la duplication explicitement (« Same shape as observability.ts — kept duplicated rather than extracted to keep the page self-contained »). Self-containment défendable v1, mais une 3ᵉ page (futurs ?) referait le même copier-coller.
  • Fix : extraire frontend/src/app/core/utils/filter-window.ts (10 lignes) avec buildFilter(fromDate, toDate, promptId, ...): FilterEnvelope | undefined et nextDayIso(date): string. Les 2 pages importent. Un test unitaire commun.

9. ObjectMapper instancié à la main dans 2 services

  • Fichiers : NarrativeObservabilityService.kt:53 (private val mapper = jacksonObjectMapper()) + NarrativeBiasService.kt:53 (idem).
  • Symptôme : Spring Boot expose déjà un bean ObjectMapper configuré (dates, nullability, modules Kotlin). En instancier 2 nouvelles à la main contourne la config globale. Inoffensif aujourd'hui (les usages se limitent à readValue<Array<String>>), mais drifte si la config Jackson évolue.
  • Fix : injecter private val mapper: ObjectMapper au lieu d'instancier.

10. NarrativeBiasQuery.normalizeInstant duplique le helper de NarrativeObservabilityQuery

  • Fichiers : NarrativeObservabilityQuery.kt:148-154 + NarrativeBiasQuery.kt:182-188 (méthode privée identique au caractère près).
  • Fix : extraire en utility commune (e.g. infrastructure/persistence/JpaInstantNormalizer.kt) quand un 3ᵉ usage apparaît. Pas la peine d'extraire pour 2 callsites.

Forces remarquables

  1. 3 reviews subagent en parallèle au lieu d'une lecture séquentielle — chaque agent a une fenêtre de contexte gérable (~95-130k tokens chacun), les 3 ont recoupé les fichiers communs sans rater de drift, et le main thread a pu arbitrer en lisant 2-3 sections ciblées du code (vérification du faux positif priceMove, vérif du caching lookupOrFallback). À encoder dans le subagent code-reviewer futur (ticket dette technique « Agent Claude code-reviewer pré-commit »).

  2. Tests as documentation tenus end-to-end sur la phase :

  3. CoherenceScorerTest : 9 tests dont les invariants de seuil (flip + flat → HIGH, flip + 5 % move → excused out of HIGH), edge cases (empty key_points → jaccard = 1, previousPrice = 0 → priceMove null), ladder sentiment (PARTIAL vs FLIPPED), normalisation jaccard (case + trim).
  4. NarrativeBiasServiceTest : 10 tests dont zero-pad 3 buckets, bias flag à exactement 60 % vs 59 %, calibration skip nulls + report contributing counts, one-fetch-per-symbol invariant, graceful degradation per-symbol, topic count par snapshot pas par occurrence, stopwords filtered.
  5. TickerNarrativePromptServiceTest : cache-miss self-heal post-patch (séquence Mockito .willReturn pour pin que la 2ᵉ lecture re-hit la DB).

  6. NarrativeObservabilityServiceTest : 12 tests (3 ajoutés pour le wiring scorer Phase 3 #2) dont oldest-row-no-coherence, regression guard sur la direction i+1 du pairing.

  7. Routing precedence pinned sur les segments littéraux (/tickers, /bias) — déclarés avant /{symbol} côté Spring controller ET côté Angular routes, avec test explicite « literal route resolves to ... and NOT to {symbol}=literal » dans le @WebMvcTest.

  8. i18n parité FR/EN intégrale — vérification empirique : ~140 nouvelles clés ajoutées sur la phase, FR et EN matchent au caractère près sur les noms de clés. Aucun hardcoded user-facing string repéré dans les diff (le ticket dette tech sur OllamaPullDialog qui restait pré-Phase 3 est toujours ouvert).

  9. Décisions design documentées — le journal et les KDoc/TSDoc des nouveaux services portent les rationales : « heuristique vs LLM-as-judge » (pour les 2 scoring), « count by snapshot vs by occurrence » (topic coverage), « page séparée vs sub-route per-prompt » (bias dashboard), « base du delta = snapshot.price vs close à T0 du bar » (timeline observabilité), « cache cross-rows vs re-parse JSON » (cohérence). Ces décisions seront lisibles par un dev qui revient dans 6 mois.

  10. Dépendances Phase 4 explicitement déclarées — la Phase 3 produit un corpus + une UI au-dessus, sans introduire de couplage avec le DAG unifié futur. Le scoring (cohérence + biais) est calibré pour rester pertinent au-dessus de PortfolioAggregation (Phase 4 #2) sans modification — le CoherenceScorer.score(current, previous) ne sait pas de quel kind de snapshot il scoreil compare des SnapshotProjection génériques. Hookup Phase 4 zero-friction.

Résidus audits précédents

  • Audit 2026-05-10-fin-phase-2.5.md — les 4 findings (Boot fragile sans ANTHROPIC_API_KEY, WatchlistService.add hors @Transactional, LlmTimeoutService dead code, validation jobId/symbol sur le SSE narratif) ont été clôturés en un commit avant le tag v0.4.0 (cf. journal-livraisons.md > Phase 2.5). Pas de carry-over Phase 3.
  • Audit 2026-05-06-fin-phase-2.md — le finding #3 (« stratégie de cache : key-prefix par adapter vs service-cache sans préfixe ») reste ouvert dans la dette technique 🟡. Pas de progrès Phase 3, mais la phase n'a pas non plus aggravé le drift (pas de nouveau cache adapter ajouté).
  • Audit 2026-05-02-revue-globale.md — résidus déjà clôturés ou re-filed dans les audits suivants. Plus pertinent ici.

Findings 🟡 dette technique toujours ouverts au moment de la revue (cf. backlog.md > Dette technique) : - Stratégie de cache key-prefix vs service (audit 2026-05-06 #3) - Agent Claude code-reviewer pré-commit (à l'image de doc-maintainer) - Onboarding doc — découper « tester en local » et « développer » - Gestion d'erreur transverse — frontend MatSnackBar + logging backend + exceptions plus honnêtes - Bar-lookup priceAtOrAfter borne basse (filed à cet audit)

Recommandation tag v0.5.0

Ready après les 3 fichiers en dirty (patch + test + backlog updates de cet audit) sont commités. Pas de bloquant restant. Les 8 résidus filed en dette technique sont tous en 🟢 (sauf bar-lookup en 🟡), aucun n'empêche un tag propre.

Suggested commit message pour le delta de cet audit : 

refactor(prompt): never cache the synthetic fallback template
ou groupé avec la dette tech si tu préfères : 
refactor(prompt): fix fallback caching + file Phase 3 review residues