Skip to content

Revue de code globale — pré-v0.5.1 (2026-05-16)

Scope : audit de la delta v0.5.0..HEAD (Phase 3 close 2026-05-14 09:08 → maintenant), 19 commits, 213 fichiers modifiés (+3 765 / −2 382 lignes). Trois grands chantiers : (1) dette technique backend — 7 refactors structurels (B1 ports infrastructure/ → domain/, B2 MarketUnavailableException → shared/UpstreamUnavailableException, B3 SymbolSearchService split en deux beans avec SymbolValidator dédié, B6 AppConfigService 14 @Value → 3 @Component data classes, B7 spring-boot/SKILL.md test section corrigée, plus 2 fixes Ollama + 1 fix lower-bound observability) ; (2) dette technique frontend — pilote Resource builders on the port sur SnapshotRepository + Suivi, split core/ en 3 axes (api/<bucket>/ + local/<bucket>/ + app-state/), bascule LlmTimeoutService + OllamaStatusService Promise → Observable, OllamaPullDialog i18n, drop /settings/prompt-preview ; (3) chantier de l'après-midi 2026-05-16 — création de 4 skills backend (kotlin-idioms, spring-boot, hexagonal-ddd, folders-structure-backend), création du subagent code-reviewer + slash command /code-review, renumérotage des phases (Phase 4 = Auth nouveau, Phase 5 = Deploy inchangé, Phase 6 = ancienne Phase 4 Vision/DAG), nettoyage substantiel de la Dette technique (démotions 🟡 → 🟢, freezes ⏳ → 🧊, closures de 2 tickets), conversion des liens README en HTML target="_blank", audit doc-maintainer Pass 2 + Pass 3.

Méthode : 3 subagents general-purpose Claude lancés en parallèle sur trois tranches cohérentes — Backend (8 commits, backend/*), Frontend (5 commits, frontend/*), .claude/ + doc-set (6 commits, .claude/* + docs/* + README.md). Chacun briefé sur les conventions ground truth (CLAUDE.md, architecture.md, ddd.md, skills pertinents) et sur l'audit précédent comme format de référence (2026-05-14-fin-phase-3.md). Sortie : punch-lists structurées (Critique / Important / Mineur) avec fichier:ligne + suggestion + rationale, plus une section Faux positifs filtrés pour traçabilité. Vérification croisée des « Critiques » par le main thread avant consolidation.

État du commit au moment de la revue : branche master, dernier commit committé 8bb04df chore: phase renumber + code-reviewer agent + dette cleanup. Working tree clean. CI verte (« la pipeline passe » confirmé par l'utilisateur avant lancement de l'audit). 14 repositories frontend répartis sur 3 axes (core/api/ 8 buckets + core/local/ 1 bucket + core/app-state/ 2 services). 9 modules backend stables (pas de nouvelle migration Flyway — V8 reste la dernière). 13 skills .claude/ + 2 subagents (doc-maintainer + code-reviewer nouveau). Tag candidat : v0.5.1 (patch — pas de phase close depuis Phase 3, la delta est dette tech + infra Claude + renumérotage prospectif).

Résumé exécutif

App globalement saine pour un tag v0.5.1. La delta v0.5.0..HEAD n'est pas une livraison de feature mais un consolidation pass : refactors structurels backend + frontend, dette technique sérieusement nettoyée (3 tickets clos, 4 démotés, 2 frozen), infra Claude étoffée (4 nouveaux skills backend + nouveau code-reviewer agent), renumérotage des phases pour mettre OAuth2 en Phase 4 avant le Deploy Phase 5. Les patterns établis ont tenu : ports outbound vivent maintenant en domain/ (hexagonal strict), exception cross-context centralisée dans shared/, @Lazy self éliminé au profit de la split-bean discipline, @Value groupés en holders cohérents, frontend split core/ 3-axes propre, pilote rxResource validé sur 1 repo.

2 Critiques identifiés — patches 1-liner chacun, à appliquer avant tag :

  1. Backend — GlobalExceptionHandler.kt:39 : le body 503 reste hardcodé à « Données de marché momentanément indisponibles » après le rename B2 MarketUnavailableException → UpstreamUnavailableException. L'exception couvre désormais 5 providers (market + news + analyst + earnings + LLM) mais l'utilisateur voit toujours « marché » sur tous les 503. Le journal-livraisons claim explicitement « message HTTP 503 inchangé (« Données momentanément indisponibles ») » — le journal s'est trompé sur le verbe « inchangé », le code a raté le sweep.

  2. Frontend — configuration.ts:288 + :323 : this.timeoutService.refresh() appelé sans .subscribe() après la migration 22fa6f5 qui a basculé l'API de Promise vers Observable. Régression runtime silencieuse : sauvegarder ou reset la clé llm.timeout-seconds ne rafraîchit plus le signal LlmTimeoutService.seconds, le label « estimation max » sous le slider reste figé jusqu'au prochain reload. Le test configuration.spec.ts:520-529 stubbe la méthode et vérifie l'appel via toHaveBeenCalledTimes(1) sans tester l'effet du retour Observable cold — le mock masque la régression.

9 Important : 3 par tranche. Côté backend : Clock seam fragile sur JobEventPublisher (default value Kotlin sans @Bean Clock), asymétrie 404 vs 5xx dans OllamaStatusService.pullModel, @Suppress("TooGenericExceptionCaught") sur WatchlistService.lookupInstrumentType. Côté frontend : pas de spec dédié sur snapshot.repository.ts (les builders rxResource testés indirectement via suivi.spec.ts), effect() dans positionsCache(...) qui capture le DestroyRef du caller sans contrainte explicite, mocks mockResolvedValue qui retournent Promise au lieu d'Observable depuis la migration. Côté doc-set : architecture.md lignes 198 + 254 décrivent toujours la stratégie de cache comme dette 🟡 alors qu'elle a été démotée à 🟢 ce matin, spring-boot/SKILL.md ligne 268 mentionne « Phase 4 aggregation/ » périmé par le renumérotage, README.md table doc manque une row Sources de données qui existe dans mkdocs.yml > nav.

~14 Mineur : polish (KDoc verbose, helpers dupliqués déjà filed en dette, imports nested companion, hint string non i18n côté ConfigTestClient, commit chore(.claude) qui livrait en fait un adapter Mock — scope creep rétrospectif), 5 refs forward-looking « Phase 4 » résiduelles dans les fichiers historiques (audits/2026-05-10 + audits/2026-05-14 + 1 occurrence ratée dans journal-livraisons.md ligne 88) — le code-reviewer agent recommande de laisser les audits historiques intacts mais de patcher l'occurrence journal qui contredit la Pass 3.

Faux positifs filtrés : 11 au total entre les 3 agents (suspicion boot fail sans @Bean Clock → en réalité Spring Kotlin honore le défaut, B6 « 14 → 6 params » comptabilité correcte, B1 SectorClassifier port présent en domain/, useValue mocks sur ports sans builders OK, drop /settings/prompt-preview complet, OllamaPullDialog i18n clés bien présentes, etc.). Trace conservée pour rendre la review reproductible.

Forces :

  • 3 reviews subagent en parallèle ont fonctionné une 2ᵉ fois — chaque agent reste dans une fenêtre de contexte gérable, les 3 vues se complètent sans recouvrement (chaque commit appartient à une seule tranche). Le main thread synthétise et arbitre les 2 Critiques.
  • Tests-as-documentation conservés : SymbolValidatorTest 4 tests sémantiques + class-level docstring, AppConfigServiceTest.newService() helper réécrit, JobEventPublisherTest avec MutableClock test seam, lower-bound guard priceAtOrAfter pin pour observability + bias. Les commit messages B-tickets décrivent le why (le Detekt threshold est descendu à 8 parce que les holders absorbent les params naturellement, pas parce qu'on est devenu plus strict pour les autres beans).
  • Discipline doc-set : Pass 2 de doc-maintainer a couvert 8 fichiers du renumérotage en une session, Pass 3 a corrigé 7 résidus drift-driven sortis du dry-run code-reviewer. Le CHANGELOG 2026-05-16 trace les 3 passes proprement.
  • Skills backend immédiatement consommés : les 4 skills créés le 2026-05-15 (kotlin-idioms, spring-boot, hexagonal-ddd, folders-structure-backend) ont déjà été validés par les refactors B1/B2/B3/B6 qui suivent (et raffinés en passant — c6b78b8 + 9f8f828 + 2517735 + d203ede + 16feff4 portent chacun un fix skill aligné). La valeur d'un skill se mesure au moment où on l'utilise pour la première fois — ici, validation immédiate.
  • Convention Resource builders on the port documentée avant généralisation : pilote SnapshotRepository + entrée dans angular-signals/SKILL.md > Resource builders live on the port itself, ticket dette 🟢 ouvert pour la généralisation aux 13 autres repos. Le pattern est défensible et les premiers consumers (Suivi) montrent le bénéfice mesurable (−40 lignes, zéro subscribe).

Backend

Périmètre couvert

  • 16feff4 refactor(backend): hoist outbound ports infrastructure/ → domain/ — ✅ 7 ports déplacés (MarketChartClient, SymbolSearchClient, SectorClassifier, NewsClient, AnalystRecommendationClient, EarningsClient, LlmClient), KDoc bracket-refs dégradés en plain backticks, imports propres, journal entry exhaustive.
  • 9f8f828 refactor(backend): rename MarketUnavailableException → shared/UpstreamUnavailableException — ⚠️ sweep correct, mais le body 503 reste « Données de marché… » alors que l'exception couvre maintenant news/analyst/earnings/LLM (cf. Critique #1).
  • 2517735 refactor(backend): split SymbolSearchService cache vs validation into two beans — ✅ @Lazy self retiré, SymbolValidator @Component dédié, WatchlistService recâblé, 4 tests dédiés + 6 sites mocks adaptés.
  • d203ede refactor(backend): group AppConfigService @Value params into 3 @Component holders — ✅ 3 data classes + service 14 → 6 params + Detekt constructorThreshold 16 → 8, AppConfigServiceTest.newService() réécrit proprement.
  • 274a47d test(backend): pin pruneStale TTL eviction and Ollama 200 status:error fail-soft — ✅ Clock seam injecté avec défaut Kotlin (Clock.systemUTC()), 2 tests défensifs ajoutés (mais voir Important #1 sur le risque Spring/Kotlin du défaut constructor).
  • 49c2094 chore(backend): align Ollama pullModel/deleteModel on \model` body field— ✅ basculename → model`, 2 lignes de body + KDoc + 2 tests assertion strings rafraîchies.
  • 5acff4d fix(backend): point Ollama 404 hint at the in-app Pull dialog — ⚠️ string change visible utilisateur, pas couverte par un test et pas i18n-isée (cf. Mineur #2).
  • 06ba7ec fix(observability): guard priceAtOrAfter lower bound so pre-chart snapshots get null deltas — ✅ guard symétrique ajouté aux deux helpers + 2 tests (observability + bias). Helper toujours dupliqué — déjà filed en dette « Coutures Phase 3 ».
  • (hors brief explicite mais présent dans le diff) c6b78b8 chore(.claude): align spring-boot test section… — ⚠️ commit chore(.claude) qui livre en réalité aussi MockLlmClient.kt (140 lignes) + son test (120 lignes) + edits sur TickerNarrativeExecutor, RoutingLlmClient, ConfigKeys. Scope creep silencieux ; le code est sain mais le commit message est trompeur.

Critique

  • backend/src/main/kotlin/com/portfolioai/shared/GlobalExceptionHandler.kt:39 — Body 503 hardcodé à « Données de marché momentanément indisponibles » après le rename B2 MarketUnavailableException → UpstreamUnavailableException.
  • Symptôme : tout 503 issu de Finnhub /company-news, /stock/recommendation, /stock/earnings, ou de Claude / Ollama, surface au frontend avec le message « Données de marché momentanément indisponibles ». Faux pour 4 des 5 providers que l'exception couvre désormais.
  • Cause : oubli pendant le sweep B2. La KDoc juste au-dessus du handler (« le user-facing message stay intentionally generic — la distinction market vs LLM vs news lives in the detail field ») se contredit avec le code. La KDoc sur l'exception elle-même (UpstreamUnavailableException.kt:5-7) parle d'un message générique « try again later ». Le journal-livraisons.md > B2 claim aussi « message HTTP 503 inchangé (« Données momentanément indisponibles ») » — le journal s'est trompé sur le verbe « inchangé », le code a raté le sweep.
  • Patch suggéré : 
    "error" to "Données momentanément indisponibles",
    Aligne le body sur l'intention déclarée et sur la chaîne déjà présente dans journal-livraisons.md > B2. Le frontend lit déjà error + detail séparément (market.repository.ts reformule via detail si présent), donc pas de breaking change frontend.

Important

  • backend/src/main/kotlin/com/portfolioai/analysis/application/JobEventPublisher.kt:36 — Clock seam avec défaut Kotlin (= Clock.systemUTC()) sur un @Component. Pas de @Bean Clock déclaré ailleurs dans le projet. Spring 6 + Kotlin Reflection devraient honorer le défaut, et BackendApplicationTests.contextLoads() couvre le boot — mais le contrat est fragile (un futur Clock bean introduit par un autre développeur capturerait silencieusement la dépendance et casserait JobEventPublisherTest.MutableClock). Suggestion : ajouter un commentaire au-dessus du constructor explicitant l'invariant (// No Clock bean is registered intentionally — Kotlin default value resolves at construction. Adding a @Bean Clock elsewhere would silently change the production behavior to track that bean's clock.) ou, plus simple, déclarer un @Bean fun clock(): Clock = Clock.systemUTC() quelque part dans analysis/AnalysisConfig.kt et basculer le constructor en injection explicite.

  • backend/src/main/kotlin/com/portfolioai/analysis/infrastructure/llm/OllamaStatusService.kt:173-179 — pullModel catch HttpStatusCodeException mais, à la différence de unloadModel/deleteModel (:131-137, :209-216), ne gère pas le 404 spécifique « model not pulled locally ». Le commit message (et la KDoc :175-178) justifient ça par « Ollama returns 500 with error: model not found rather than 404 ». OK mais asymétrie surprenante — si Ollama un jour aligne /api/pull sur 404, le user verra « HTTP 404 from Ollama » au lieu du hint utile. Le test pullModel surfaces 5xx from Ollama as fail-soft ne pin que la branche 500. Suggestion : soit ajouter le même branchement 404 défensif (1 if), soit ajouter une note de test qui pin explicitement « si Ollama jamais renvoie 404, on retombe sur le message générique ».

  • backend/src/main/kotlin/com/portfolioai/watchlist/application/WatchlistService.kt:127-139 — @Suppress("TooGenericExceptionCaught") sur lookupInstrumentType(symbol) qui catch (e: Exception) autour de tickerService.load(...). Ce hint est inscrit dans le backlog ticket « Gestion d'erreur transverse — frontend MatSnackBar + logging backend + exceptions plus honnêtes » (Volet 3) qui demande explicitement de resserrer ce catch sur HttpClientErrorException/UpstreamUnavailableException. Ne bloque pas le tag, mais inscrire un TODO inline ne ferait pas de mal pour que le lecteur soit prévenu que ce site est ciblé.

Mineur / À discuter

  • backend/src/main/kotlin/com/portfolioai/config/application/AppConfigService.kt:42 — cacheTtlDefault est resté en @Value standalone tandis que les 12 autres params YAML sont regroupés en 3 holders. Le KDoc le justifie (« doesn't bundle naturally with any of the three groups »), mais quand on lit le constructor on cligne — soit ajouter un 4ᵉ holder MarketCacheDefaults minuscule pour homogénéité (overkill), soit laisser comme tel mais raboter le commentaire de 3 lignes à 1.

  • backend/src/main/kotlin/com/portfolioai/config/infrastructure/ConfigTestClient.kt:226 — Le hint Pull dialog est une chaîne anglaise embarquée dans une TestConfigResult qui sera affichée tel quel au user. La convention projet veut qu'aucune chaîne user-facing ne soit hardcodée (passer par ngx-translate côté front). Trade-off : ici la chaîne est forgée backend et passe par le message du DTO ; pour respecter la convention il faudrait un code d'erreur (ex. OLLAMA_MODEL_NOT_FOUND_PULL_HINT) que le front traduit. Pas critique mais pin la dette « Gestion d'erreur transverse » qui prévoit justement les enum reasons.

  • backend/src/main/kotlin/com/portfolioai/analysis/application/NarrativeBiasService.kt:248-254 + NarrativeObservabilityService.kt:205-211 — Le helper priceAtOrAfter est dupliqué entre les 2 services (la KDoc l'avoue : « Mirror of …, kept duplicated to keep services independent »). Le fix B7 (06ba7ec) a dû patcher les 2 endroits. Candidat à extraction dans market/domain/BarLookups.kt (top-level fun) — déjà filed dans dette « Coutures Phase 3 » mais ce delta y replonge en cumulant la duplication.

  • backend/src/main/kotlin/com/portfolioai/market/application/SymbolValidator.kt:3 — Import import com.portfolioai.market.application.SymbolSearchService.Companion.DEFAULT_LIMIT traverse une nested companion. Kotlin idiomatic, mais légèrement inhabituel — un private const val DEFAULT_LIMIT = 10 dans SymbolValidator (avec un commentaire « must stay aligned with SymbolSearchService.DEFAULT_LIMIT for shared cache hit ») serait plus auto-documenté. À voir si la centralisation actuelle paye réellement.

  • backend/build.gradle.kts — Le drop de l'allowlist no-wildcard-imports est verbose (~25 lignes de commentaire). OK pour un changement structurel, à raboter à 5-6 lignes la prochaine fois que ce bloc est touché.

  • Commit c6b78b8 — chore(.claude) qui livre un adapter Mock entier (260 lignes code+tests). À titre rétrospectif, le commit aurait gagné à être splitté en deux : (1) chore(.claude): align skills, (2) feat(backend): MockLlmClient for keyless onboarding. Pas actionnable rétroactivement, simple note pour le futur.

Faux positifs filtrés

  • Boot avec JobEventPublisher(Clock) sans @Bean Clock — premier réflexe : « Spring va échouer à autowire Clock puisque c'est un type non-Spring ». Vérification : BackendApplicationTests.contextLoads() est @SpringBootTest smoke-test et tourne en CI ; Spring 6 + Kotlin reflection honore les valeurs par défaut Kotlin via KotlinDetector + BeanUtils.findPrimaryConstructor ; et le test JobEventPublisherTest instancie JobEventPublisher() sans arg (zero-arg overload synthétique fourni par Kotlin) — qui passe. Donc le boot est OK. Reclassé en Important #1 (commentaire d'invariant + bean clock explicite) plutôt qu'en Critique.

  • B6 « 14 → 6 params » — Le diff exhibe 6 params au constructor mais le commit message + le journal claiment « 14 → 6 ». Vérif : 2 deps (repository, eventPublisher) + 3 data classes (secrets, dataProviders, llm) + 1 standalone @Value cacheTtlDefault = 6 ✓. La comptabilité est correcte.

  • Suspicion B1 ports manquants — premier comptage : KDoc de SectorClassifier.kt mentionne uniquement MockSectorClassifier et FinnhubSectorClassifier, mais le brief liste SectorClassifier comme port distinct. Vérif : git show 16feff4 -- '**/SectorClassifier.kt' → bien déplacé en domain/, et RoutingSectorClassifier existe en infrastructure/. Les 7 ports sont bien tous présents.

Verdict

needs-fix pour la tranche backend.

Le seul finding bloquant est le body 503 « Données de marché… » resté en place après le rename B2 — patch ~1 ligne, contredit explicitement le journal et la KDoc du handler. Les 3 Important sont défensifs (clock seam, asymétrie pullModel 404, @Suppress étendu) et peuvent stationner en backlog si l'utilisateur préfère taguer rapidement. La tranche est globalement très propre : B1/B2/B3/B6 respectent leurs intentions journal, tests robustes (Clock injecté, 200-status:error pin, lower-bound guard pin), aucun wildcard import, aucun this.method() AOP-bypass, aucune nouvelle migration Flyway (V8 reste la dernière).

Frontend

Périmètre couvert

  • 1fbe90d refactor(frontend): port-side rxResource builders — Snapshot pilot — ⚠️ pilote solide mais sans spec dédié sur snapshot.repository.ts (testé indirectement via suivi.spec.ts), et l'effect() dans positionsCache(...) capture le DestroyRef du caller sans contrainte explicite documentée.
  • a8f8a48 refactor(frontend): split core/ on 3 axes — api/, local/, app-state/ — ✅ split propre, chacun des 14 repositories est dans la catégorie attendue, providers.ts wire les 14 explicitement.
  • 22fa6f5 refactor(frontend): LlmTimeout + OllamaStatus services Promise → Observable — 🚨 régression runtime silencieuse sur configuration.ts (cf. Critique #1).
  • b4aa264 fix(frontend): route OllamaPullDialog fallback strings through TranslateService — ✅ 3 fallback strings basculées sur translate.instant(), clés présentes verbatim dans fr.json:502-503 + en.json:502-503.
  • e0c43ff refactor(settings): drop the /settings/prompt-preview page now that the Phase 3 editor covers it — ✅ suppression complète (composant + spec + route + sub-sidenav entry + i18n keys orphelines purgées).

Critique

  • frontend/src/app/features/settings/configuration/configuration.ts:288 et :323 — this.timeoutService.refresh() est appelé sans .subscribe() après la migration Promise → Observable (22fa6f5).
  • Symptôme : sauvegarder ou reset la clé llm.timeout-seconds ne rafraîchit plus le signal LlmTimeoutService.seconds, donc le label « estimation max » sous le slider reste figé sur la valeur de boot jusqu'au prochain reload. La docstring de LlmTimeoutService (lignes 8-12) garantit pourtant explicitement « refreshed on save so the label updates immediately after a slider drag ». Régression runtime invisible aux tests parce qu'elle vit uniquement à l'exécution.
  • Cause : refresh() était async refresh(): Promise<void> (hot, démarrait à l'appel) et est devenu refresh(): Observable<void> (cold, n'exécute rien sans subscribe). Le commit 22fa6f5 a modifié refresh() mais a oublié de toucher configuration.ts ; le test configuration.spec.ts:520-529 stubbe la méthode et vérifie uniquement qu'elle a été appelée (toHaveBeenCalledTimes(1)), donc le mock masque la régression — l'invocation a bien lieu en prod aussi, mais retourne un Observable jeté.
  • Patch suggéré : 
    if (key === LLM_TIMEOUT_KEY) this.timeoutService.refresh().subscribe();
    aux deux callsites (configuration.ts:288 et :323), et durcir le test pour le pointer (par ex. faire émettre un Observable mock et assert que la value du signal a bougé, ou stubber refresh: vi.fn(() => of(undefined)) + asserter que subscribe a tiré).

Important

  • frontend/src/app/core/api/portfolio/snapshot.repository.ts:51-55 — l'effect() dans positionsCache(...) capture le DestroyRef de l'injection context dans lequel le builder est appelé. Aujourd'hui c'est OK parce que le pilote Suivi invoque la méthode en field initialiser (DI scope = component). Si un consommateur futur appelle positionsCache(...) depuis un handler / un providedIn: 'root' service / un runInInjectionContext mal placé, l'effect leakera ou throwera selon le contexte. La KDoc de la convention (skill angular-signals) doit explicitement contraindre l'appelant : « à invoquer dans un injection context lié au DestroyRef souhaité — typiquement field initialiser de composant ». Filé en doc-debt à minima, idéalement contrainte runtime via assertInInjectionContext() au début du builder.

  • Pas de snapshot.repository.spec.ts — les deux builders du port (allResource, positionsCache) ne sont testés qu'indirectement à travers suivi.spec.ts + MockSnapshotRepository. Pour un pattern qui va être généralisé aux 13 autres repos (cf. backlog 🟢 « Resource builders généralisés »), un spec qui pin spécifiquement le comportement des builders contre une fake extends SnapshotRepository (id rotation, cache hit, undefined trigger → idle, accumulator effect) limiterait la dette quand les autres repos copieront la convention. Test bonus à 30 min.

  • configuration.spec.ts:204 et :233 — mockResolvedValue(undefined) (renvoie un Promise) pour LlmTimeoutService.refresh et OllamaStatusService.refresh/unload/delete, alors que la production renvoie maintenant un Observable. Les tests passent par chance (les composants n'introspectent pas le retour), mais la divergence test ↔ runtime est exactement le pattern qui a permis au bug critique ci-dessus de passer. À harmoniser sur vi.fn(() => of(undefined)).

Mineur / À discuter

  • frontend/src/app/core/api/analysis/llm-timeout.service.ts:50-51 — catchError(() => of(undefined)) puis map(() => undefined). Le map final ne sert qu'à typer le retour en Observable<void> ; tap() ne change pas le type d'output, donc en l'absence d'erreur on émet déjà un ConfigEntry[]. Le map(() => undefined) force le contrat void, OK, mais l'enchaînement catchError + map est redondant — catchError(() => of<void>(undefined)), map(() => undefined as void) ou directement un mapTo(undefined) suffirait. Pure lisibilité.

  • frontend/src/app/core/api/analysis/ollama-status.service.ts:58-61 — startPolling appelle this.refresh().subscribe() puis setInterval(() => this.refresh().subscribe(), ...). Les subscriptions internes n'ont pas de takeUntilDestroyed (service providedIn: 'root' → pas de destroy). Si jamais la page panel mount/unmount en rafale et que le polling tick déclenche un refresh() après un stopPolling(), on a une race bénigne (le snapshot post-stop écraserait le _status signal). Pas un bug aujourd'hui — stopPolling clear l'interval avant qu'un tick supplémentaire ne soit déclenché. À garder en tête si on ajoute une 2ᵉ surface d'appel.

  • Mock convention — la skill angular-signals impose useClass extends Port pour les ports à builders hérités. Aujourd'hui seule SnapshotRepository est concernée et suivi.spec.ts applique bien la règle. Les 13 autres repos restent en useValue ailleurs (ticker.spec.ts, dashboard.spec.ts, configuration.spec.ts, bias.spec.ts, etc.) — pas un problème puisque ces ports n'ont pas de builders, mais à pin dans le backlog 🟢 « Resource builders généralisés » : chaque migration de repo doit aussi convertir ses specs useValue → useClass extends.

  • frontend/eslint.config.js — l'ajout no-unused-vars avec argsIgnorePattern: '^_' est sain et motivé par le commentaire ; rien à dire.

  • frontend/src/app/core/api/portfolio/snapshot.repository.ts:1-3 — les imports sont map, Observable puis rxResource puis Signal, effect, signal répartis sur trois lignes. Pure cosmétique mais le regroupement @angular/core vs rxjs vs @angular/core/rxjs-interop mérite d'être stable pour les autres ports qui copieront la convention. (Pas un blocker.)

Faux positifs filtrés

  • useValue dans OllamaStatusService.spec et LlmTimeoutService.spec — les ports OllamaStatusRepository / ConfigRepository n'ont pas de builders hérités (juste des méthodes abstraites). Un mock useValue qui est en fait une class extends Port instanciée fonctionne (les méthodes sont sur l'instance), et c'est ce que font les deux specs. Pas une violation de la convention.

  • Réorganisation core/ 3 axes — chacun des 14 repositories listés dans providers.ts est bien dans la catégorie attendue (HTTP → api/, localStorage annotation/ → local/, signal services → app-state/). providers.ts wire les 14 explicitement, l'ordre est cohérent avec le commentaire d'introduction. Aucune omission détectée.

  • Drop /settings/prompt-preview — grep verbatim sur prompt-preview / promptPreview / PromptPreview dans frontend/src/, frontend/public/i18n/*.json, app.routes.ts, settings.html, settings.ts et app.html : zéro hit. Suppression complète, pas de clé i18n orpheline.

  • OllamaPullDialog i18n — les deux clés settings.configurationPage.ollamaStatus.pullDialog.errors.pullFailed et .deleteFailed sont présentes verbatim dans en.json:502-503 et fr.json:502-503. Le fix b4aa264 est complet.

Verdict

needs-fix pour la tranche frontend.

Le bug timeoutService.refresh() sans .subscribe() est une régression runtime silencieuse introduite par le commit 22fa6f5 ; à patcher avant d'apposer v0.5.1. Les deux autres « Important » (KDoc contrainte injection context sur les builders + harmonisation des mocks Promise→Observable) sont du polish à attaquer dans la foulée. Le pilote Snapshot est solide côté implémentation et tests ; le split core/ 3 axes est propre et la migration Ollama/i18n est complète.

.claude/ + doc-set

Périmètre couvert

  • 5aae398 chore(.claude): overhaul skills + drop effect() from frontend services — ✅ 4 nouveaux skills backend posés + refresh angular-signals aligné sur set-site convention.
  • c6b78b8 chore(.claude): align spring-boot test section with reality + add perf guidance — ⚠️ corrige le claim @SpringBootTest, mais réintroduit une ref forward-looking « Phase 4 aggregation/ » périmée le même jour (cf. Important #2).
  • 5ddd339 chore(backlog): file .claude/ overhaul + Kotlin/Spring/hexa-DDD skills as tech debt — ✅ ticket préparatoire, depuis clos.
  • 1c78f72 docs(backend): document the two cache models explicitly (key-prefixed vs service-level) — ⚠️ Modèle A/B documenté côté architecture.md, mais le statut 🟡 → 🟢 du ticket cache n'est pas propagé dans la section « Décisions techniques notables » (cf. Important #1).
  • f4113b4 docs(backlog): file mkdocs numbering + TOC and EN translation as tech debt — ✅ 2 tickets posés, depuis l'un freezé 🧊.
  • 8bb04df chore: phase renumber + code-reviewer agent + dette cleanup — ⚠️ mega-commit globalement propre ; deux refs forward-looking « Phase 4 » oubliées et le README ne pointe pas sur Sources de données qui vit dans mkdocs.yml.

Critique

Aucun blocker pour v0.5.1 côté .claude/ + doc-set. Les drifts identifiés ne mentent pas à un nouveau dev sur l'état actuel, ils trahissent juste la propagation incomplète du renumérotage de phases.

Important

  • docs/technique/architecture.md:198 et docs/technique/architecture.md:254 — la « Stratégie de cache » est toujours décrite comme « dette technique 🟡 ouvert(e) » dans les deux endroits. Le backlog.md > Dette technique l'a explicitement démotée à 🟢 Basse le 2026-05-16 (mention « Pourquoi 🟢 Basse : démoté 2026-05-16 — cosmétique »), et l'audit Pass 2 du CHANGELOG mentionne ce nettoyage parmi les démotions 🟡 → 🟢. Symptôme : un lecteur de architecture.md croit l'effort nécessaire à court terme alors que le backlog l'a explicitement repoussé. Patch : remplacer les deux occurrences 🟡 par 🟢 (ou « dette technique tracée en 🟢 Basse »).

  • .claude/skills/spring-boot/SKILL.md:268 — la section perf des tests évoque « if Phase 4 introduces more on aggregation/ ». Avec le renumérotage du 2026-05-16 (chore 8bb04df), aggregation/ vit en Phase 6 (Vague 1 #2 « Réintégration Phase 0 — PortfolioAggregation »). Le skill a été écrit l'avant-veille du renumérotage et n'a pas été repassé. Le code-reviewer agent (.claude/agents/code-reviewer.md:98) classe explicitement les .claude/skills/* parmi les « fichiers vivants » à patcher si périmés (par opposition à journal-livraisons.md + audits/* historiques). Patch : « Phase 6 introduces more on aggregation/ ».

  • README.md ↔ mkdocs.yml — la table doc du README liste 14 rubriques, mais projet/sources.md n'y figure pas alors que mkdocs.yml:25 l'expose dans la nav (« Sources de données »). Le doc-maintainer.md:52 énonce explicitement la règle d'audit bidirectionnel : « nav additions … might warrant a README row too ». Patch suggéré : ajouter une row <a href="https://jv3n.github.io/trade/projet/sources/">Sources de données</a> | Twelve Data, Finnhub — endpoints, quotas, mocks entre les rows « Journal des livraisons » et « Conventions de commit ».

Mineur / À discuter

  • docs/projet/journal-livraisons.md:88 — entrée historique « Lifecycle de position OPEN/CLOSED » contient encore (snapshots Phase 3, croisement Phase 4). Le « croisement » est la Phase 6 Vague 3 #5 depuis le renumérotage. Pass 3 du CHANGELOG a explicitement décidé que les forward-looking refs du journal doivent rester cohérentes avec le backlog vivant, et a patché 6 occurrences — celle-ci a échappé au sweep. Borderline : « croisement Phase 4 » est dans une parenthèse d'explication de design (forward-looking), pas dans un libellé de section historique. À arbitrer : laisser tomber par fatigue, ou aligner sur la convention Pass 3.

  • docs/projet/audits/2026-05-14-fin-phase-3.md:73, 144 — 2 refs forward-looking « PortfolioAggregation Phase 4 ». Le code-reviewer.md:98 documente explicitement que les audits/* sont des « snapshots datés » et qu'une ref périmée s'y signale « À discuter avec la note « ref historique, à arbitrer », jamais en Bloquant ». Ne pas patcher mais signaler — le contrat du fichier est immuable par date.

  • docs/projet/audits/2026-05-10-fin-phase-2.5.md:53, 196, 218 — 3 refs « Phase 4 cron / job orchestration » et « DAG cible Phase 4 ». Même cas que ci-dessus, historique daté.

  • .claude/skills/hexagonal-ddd/SKILL.md:174 — « Dette technique ticket #1 tracks homogenising it ». Le backlog.md n'utilise pas une numérotation de ticket (#1, #2…) — l'identifiant relève d'un autre projet ou d'une convention abandonnée. Le ticket existe bien sous le libellé « Stratégie de cache : homogénéiser les six caches sur le pattern service-sans-préfixe ». Patch suggéré : remplacer « ticket #1 » par « le ticket « Stratégie de cache » de la Dette technique ».

  • .claude/CLAUDE.md Skills and hooks table — la table ne référence pas le dossier .claude/agents/ directement. Les deux agents (code-reviewer.md, doc-maintainer.md) sont mentionnés indirectement via leurs slash commands (rows skills/code-review/ et skills/doc-maintainer/). C'est un choix éditorial défendable (les agents ne sont pas auto-invocables, on passe par le skill) mais un lecteur qui cherche directement où trouver les agents ne le voit pas. À discuter — soit ajouter une row agents/ qui pointe vers les deux fichiers, soit laisser tel quel et compter sur les references croisées dans les SKILL.md.

  • docs/CHANGELOG.md 2026-05-16 Pass 3 — l'entrée mentionne « 6 refs Phase 4 corrigées en Phase 6 dans 5 entrées datées 2026-05-07 → 2026-05-14 » du journal. Reste 1 ref (Lifecycle OPEN/CLOSED ligne 88) qui contredit le claim « toutes les forward-looking refs sont corrigées ». La discordance est sub-bullet, pas grave, mais elle illustre la limite de complétude du sweep.

Faux positifs filtrés

  • Pas de blocage sur code-reviewer.md exception gros diffs : ajoutée dans la Pass 3 sur retour du dry-run, et bien testée conceptuellement même si pas exercée — pas un drift, c'est une convention rédigée préventivement.

  • Pas de signalement sur les HTML links dans le README : tous les 18 liens (badges + table doc) sont bien en <a target="_blank" rel="noopener">, conforme à la conversion 8bb04df. Aucun lien relatif .md résiduel.

  • Pas de signalement sur l'absence des nouveaux skills backend dans developper.md : ces skills sont du tooling Claude interne, pas du contenu onboarding humain ; developper.md n'a pas vocation à les référencer. Le doc-maintainer.md ne l'exige pas non plus.

  • Pas de signalement sur la mention MarketUnavailableException dans audits/2026-05-14-fin-phase-3.md : le rename MarketUnavailableException → shared/UpstreamUnavailableException est postérieur (commit 9f8f828) ; l'audit historique daté reflète l'état du code à la date du 2026-05-14, fidèle au contrat des audits/*.

Verdict

mergeable pour la tranche .claude/ + doc-set.

Aucun blocker pour v0.5.1. Deux corrections importantes (architecture.md × 2 statuts cache 🟡 → 🟢, spring-boot/SKILL.md ref Phase 4 → Phase 6) et un nice-to-have (README.md row Sources de données) — ~10 minutes de patch total. Les forward-looking refs résiduelles dans les fichiers historiques (audits + 1 oubli journal) sont défendables par la règle « historique daté immuable » du code-reviewer.md ; le seul vrai oubli journalistique est ligne 88 sur le « croisement Phase 4 ». Le mega-commit 8bb04df qui porte le renumérotage est globalement bien propagé — 95 % de couverture, les 5 % restants n'introduisent aucune confusion grave pour un lecteur en 2026-05-16.

Verdict global et prochaines étapes

État pour v0.5.1 : needs-fix global, dominé par les 2 régressions runtime (1 backend, 1 frontend) — chacune un patch 1-liner. Une fois les 2 Critiques traités, la tranche .claude/ + doc-set est déjà mergeable et les Important sont arbitrables.

Action plan suggéré

  1. Patcher les 2 Critiques immédiatement (10 min) :
  2. GlobalExceptionHandler.kt:39 — body 503 → « Données momentanément indisponibles ».
  3. configuration.ts:288, :323 — ajouter .subscribe() aux deux callsites, durcir le test pour pin la valeur du signal après refresh.
  4. Arbitrer les 9 Important :
  5. Backend (Clock seam, asymétrie 404 Ollama, @Suppress watchlist) — soit patch maintenant (15 min total), soit file en backlog. Le 1er (Clock) est le plus mérite d'un patch immédiat (commentaire d'invariant 2 lignes).
  6. Frontend (KDoc contrainte injection, spec snapshot.repository.ts dédié, mocks Promise→Observable) — patch dans la foulée recommandé, surtout le 3ᵉ qui a directement causé le Critique frontend.
  7. Doc-set (architecture.md cache 🟡 → 🟢, spring-boot/SKILL.md « Phase 4 », README row Sources de données) — ~10 min total, à patcher avant le tag pour cohérence.
  8. Filer les Mineurs en backlog : la plupart relèvent de la dette « Coutures Phase 3 » (helper dupliqué, etc.) ou de polish à faire à l'occasion.
  9. Une fois les patches faits : update du backlog si une feature change de statut + entrée CHANGELOG 2026-05-16 sous-bloc « Pass 4 — pré-tag v0.5.1 patches » + tag annoté v0.5.1.

Décisions retenues du dry-run code-reviewer (avant audit)

Pour mémoire : ce rapport a aussi été nourri par les conventions agentes définies ce matin dans .claude/agents/code-reviewer.md, en particulier (a) le contrat « les journal-livraisons.md + audits/* historiques sont des snapshots datés », (b) le contrat « les .claude/skills/* sont des fichiers vivants à patcher si périmés », (c) la frontière « le code-reviewer ne couvre pas le drift rédactionnel pur du doc-set (périmètre doc-maintainer) ». Ces 3 contrats ont guidé les arbitrages « Important vs Mineur » pendant cet audit.