Repository navigation
docs: duplicate detection is TF-IDF, not semantic - #2
Merged
Merged
Conversation
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.
La documentation de ce dépôt annonce une détection sémantique de doublons et un passage au modèle neuronal à portée d'une variable d'environnement. Le code ne tient ni l'un ni l'autre. Cette PR corrige la documentation ; elle ne touche pas au comportement, qui est délibéré.
Le constat, dans le code
src/agents/pattern_classifier.py:112:self._embedder = Embedder(force_tfidf=True) # TF-IDF always (CI-safe).src/core/embedder.py:112-115:use_neural = not force_tfidf and os.environ.get("USE_NEURAL_EMBEDDINGS", "") == "true"— avecforce_tfidf=True, la condition est toujours fausse.Embedderdu dépôt (grepsur tout le code, tests compris). La variableUSE_NEURAL_EMBEDDINGSn'a donc aucun effet sur la détection de doublons, qui tourne toujours sur de la similarité lexicale.Le README le disait déjà, d'ailleurs, dans ses limites connues : « TF-IDF capte la similarité lexicale, pas la sémantique fine ». C'était l'accroche, deux lignes plus haut, qui promettait l'inverse.
Changements
README.md/README.en.md, accrocheEmbeddersait le charger, mais le classifieur ne l'utilise pasdocs/ADR-002docker-compose.ymlLa décision d'ADR-002 n'est pas réécrite : TF-IDF par défaut, pour la reproductibilité et une CI sans réseau, reste le bon choix. Ce qui était faux, c'est la conséquence qui annonçait un basculement à une variable près. L'amendement daté garde la trace de l'écart plutôt que de le faire disparaître.
Vérifié
PatternClassifierest le seul appelant qui construit unEmbedder: vérifié pargrepsursrc/ettests/.TfidfEmbedder._DIM = 512etNeuralEmbedder.dim → 384, lues danssrc/core/embedder.py.Non fait
Contexte
Écart repéré en vérifiant un article du site qui citait ce dépôt. La page d'accueil et l'article ont été corrigés de leur côté (bazanjeremy.github.io#34) ; cette PR ferme le même écart ici.