Skip to content

fix: only vector classical rag#13

Merged
Kaiohz merged 1 commit into
mainfrom
feat/fix-response-rag
Apr 28, 2026
Merged

fix: only vector classical rag#13
Kaiohz merged 1 commit into
mainfrom
feat/fix-response-rag

Conversation

@Kaiohz
Copy link
Copy Markdown
Contributor

@Kaiohz Kaiohz commented Apr 26, 2026

No description provided.

@Kaiohz
Copy link
Copy Markdown
Contributor Author

Kaiohz commented Apr 26, 2026

🔍 Code Review

📝 Analyse des changements

Ce fix modifie le comportement des scores optionnels dans ClassicalChunkResponseSchema:

- bm25_score: z.number().optional()
+ bm25_score: z.number().nullish()

Pourquoi c'est important:

  • .optional() accepte undefined mais rejette null
  • .nullish() accepte les deux (undefined ET null)
  • Quand l'API backend renvoie explicitement null (ex: mode vector-only sans BM25), le schema doit l'accepter

✅ Points positifs

  • Changement minimal et focalisé (single responsibility)
  • Tests mis à jour pour refléter le nouveau comportement
  • Convention de commit respectée

📊 Score: 8/10

Critère État
Code correct
Tests à jour
Changement ciblé
Description PR ⚠️ Manquante
Titre descriptif ⚠️ Pourrait être plus clair

💡 Suggestions

  1. Ajouter une description à la PR avec le contexte:

    • L'API renvoie null pour les scores non calculés en mode vector-only
    • Le schema rejetait ces null car optional() != nullish()
  2. Titre alternatif suggéré:

    "fix(schema): allow null scores in ClassicalChunkResponse for vector-only mode"

  3. Considérer un test edge case supplémentaire:

    • Chunk avec tous les scores présents (pas null)
    • Valider que le schema accepte aussi bien null que des valeurs numériques

Verdict: Prêt à merger. Le fix est correct et les tests couvrent le cas. 🚀

@Kaiohz Kaiohz merged commit 70c508e into main Apr 28, 2026
1 check passed
@Kaiohz Kaiohz deleted the feat/fix-response-rag branch April 28, 2026 08:54
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.

1 participant