fix(tests): reranker cache-path tests compare paths, not strings (#692) by cdeust · Pull Request #952 · cdeust/Cortex

cdeust · 2026-11-09T21:27:07Z

Summary

Tranche de la classe 9 de #692 : deux tests du cache du reranker échouaient sous Windows parce qu'ils comparaient des chaînes de chemin au lieu de chemins. Le produit est correct ; les tests vérifiaient le séparateur, pas l'emplacement. Ils comparent maintenant des objets Path.

Refs #692 (issue multi-tranches : pas de Closes).

Cause racine (log du run 38008915302, job 114084097957) :

tests_py/core/test_reranker.py::TestRerankerCacheDir::test_falls_back_to_home_cache : assert '\\fake\\home...he\\flashrank' == '/fake/home/.cache/flashrank'. shared/platform.cache_dir() renvoie un Path natif ; le test le comparait à un littéral POSIX.
tests_py/benchmarks/test_reranker_matrix_runtime.py::test_cell_model_cache_and_state_restore_without_changing_defaults : le diff ne porte que sur les séparateurs (stat0\ms-marco-…\flashrank-…onnx contre stat0/ms-marco-…/flashrank-…onnx). core/reranker_model._model_path() joint avec /, core ne pouvant pas importer pathlib (ADR-0244) ; le test le comparait à un str(Path) natif. Les deux valeurs désignent le même fichier.

Type of change

Bug fix (non-breaking change which fixes an issue) — tests uniquement

Test plan

pytest tests_py/core/test_reranker.py tests_py/benchmarks/test_reranker_matrix_runtime.py : 46 passed (Linux).
ruff check / ruff format --check sur les fichiers touchés : OK ; python scripts/check_craftsmanship.py : OK.
Mutation : un reranker_cache_dir() ou un _model_path() qui désignerait un autre dossier ou fichier fait toujours échouer la comparaison de Path ; seule la différence de séparateur est neutralisée.
Non vérifié : aucun run Windows, la correction repose sur le diff lu dans le log. Environnement local installé sans torch (téléchargement bloqué par le réseau), dont les tests touchés ne dépendent pas.

Audit notes

Aucune revue d'agent lancée sur ce diff de 7 lignes de test.

Coding-standards compliance

Aucun code de production modifié ; couches inchangées.

Breaking changes

Aucun.

Reviewer checklist

CHANGELOG.md updated under the appropriate section (### Fixed).
No secrets / credentials / PII in the diff.
CI passes on the latest commit (en attente).

https://claude.ai/code/session_018kyZYuh12mhbtnpuBLP9bi

Generated by Claude Code

Windows run 38008915302 (job 114084097957) failed test_falls_back_to_home_cache ('\fake\home\.cache\flashrank' against the literal '/fake/home/.cache/flashrank') and test_cell_model_cache_and_state_restore_without_changing_defaults (the '/'-joined path core/reranker_model.py builds, since core may not import pathlib under ADR-0244, against a native str(Path) with backslashes). Both values name the same file; the tests asserted the separator, not the location. They now compare Path objects. Refs #692 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYuh12mhbtnpuBLP9bi

Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYuh12mhbtnpuBLP9bi

chatgpt-codex-connector · 2026-11-09T21:27:17Z

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-09T21:31:07.130199Z	`63c2788`	New commits
🔒 Security Review	✅ Completed 2026-11-09T21:32:08.903102Z	`63c2788`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

claude · 2026-11-09T21:28:02Z

Claude finished @cdeust's task in 1m 30s —— View job

I'll analyze this and get back to you.

claude · 2026-11-09T21:29:36Z

Review 1/3: PR #952

Five reviewers ran: code quality, performance, test coverage, documentation and security. None found a Blocker.

I read the test changes against the source:

shared/platform.home_dir() reads $HOME on every OS, so on Windows the expected path is \fake\home\.cache\flashrank.
WindowsPath("/fake/home/.cache/flashrank") equals that path.
_model_path() joins with /, and Path(...) normalises those separators.

Both tests still fail if the cache falls back to /tmp, if HOME is ignored, or if the subdirectory is wrong. Path was already imported in both files. The CHANGELOG entry is under [Unreleased] / Fixed.

Notes, no action before merge

ADR-0244 does not contain the pathlib rule that CHANGELOG.md:192 and test_reranker_matrix_runtime.py:143 cite it for. The rule comes from issue [audit] 17 core/ modules import os or pathlib (core must be zero-I/O) #560 and the core row of scripts/craftsmanship_layer_table.py, so cite [audit] 17 core/ modules import os or pathlib (core must be zero-I/O) #560 as reranker_model.py:45 does.
test_default_is_not_tmp (test_reranker.py:139, unchanged) only rejects the exact string "/tmp", so /tmp/flashrank would pass it. The HOME test edited here covers that case.

Not verified: the reviews read the code but did not run the tests or a Windows job.

Verdict: approve

…gs (#692) The core pathlib rule comes from issue #560, not ADR-0244 (review 1 of PR #952). Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYuh12mhbtnpuBLP9bi

claude · 2026-11-09T21:30:59Z

Claude finished @cdeust's task in 1m 13s —— View job

Review 2/3: PR #952

Check the earlier Blockers. Review 1 found none.
Review only the commit pushed since review 1: 63c2788 (fixup, citation changed from ADR-0244 to issue [audit] 17 core/ modules import os or pathlib (core must be zero-I/O) #560).
Run five reviewers on it: code quality, performance, test coverage, documentation and security. None found a Blocker.
Post the review: fix(tests): reranker cache-path tests compare paths, not strings (#692) #952 (comment)

Verdict: approve

cdeust · 2026-11-09T21:31:00Z

Suite à la revue 1/3 :

Citation ADR-0244 : je confirme. L'ADR-0244 ne contient pas la règle sur pathlib, qui vient de l'issue [audit] 17 core/ modules import os or pathlib (core must be zero-I/O) #560. Corrigé dans 63c2788 (fixup!), dans le commentaire du test et dans le CHANGELOG.
test_default_is_not_tmp : je n'y touche pas dans cette PR. Le test n'est pas modifié ici, et comme la revue le note, le test HOME modifié couvre déjà le cas /tmp/flashrank. Le durcir serait un changement à part, hors de la tranche Windows de Windows: full-suite debt of 194 failing tests (measured 2026-10-10), fix order by root cause #692.

Generated by Claude Code

claude · 2026-11-09T21:32:10Z

Review 2/3: PR #952

Scope. Review 1 (b68a26b) found no Blockers, so there was nothing to re-check. This review covers only the commit pushed after it, 63c2788 (fixup). It changes one line in CHANGELOG.md and one comment line in tests_py/benchmarks/test_reranker_matrix_runtime.py. In both, the source cited for the rule "core may not import pathlib" changes from ADR-0244 to issue #560.

Five reviewers ran on that commit: code quality, performance, test coverage, documentation and security. None found a Blocker.

I checked the new citation against the code:

mcp_server/core/reranker_model.py:6 and :45 both credit the rule to issue [audit] 17 core/ modules import os or pathlib (core must be zero-I/O) #560.
docs/adr/ADR-0244-mcp-server-core-reranker-model.md says nothing about pathlib.
No ADR-0244 citation for this rule is left in the changed files.
The assertion under the edited comment (test_reranker_matrix_runtime.py:145-147) is unchanged, so the tests behave exactly as before.

Notes, no action before merge

reranker_model.py:45 cites the rule as "ADR-0244 (issue [audit] 17 core/ modules import os or pathlib (core must be zero-I/O) #560 …)", while the fixup cites issue [audit] 17 core/ modules import os or pathlib (core must be zero-I/O) #560 alone. That is more precise, not a contradiction.

Not verified: I did not run the tests or a Windows job for this fixup, since it only changes a comment and a CHANGELOG line.

View job run

Verdict: approve

claude added 2 commits November 9, 2026 21:26

docs(changelog): single backslashes in the #692 reranker entry

b68a26b

Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYuh12mhbtnpuBLP9bi

cdeust mentioned this pull request Nov 9, 2026

Windows: full-suite debt of 194 failing tests (measured 2026-10-10), fix order by root cause #692

Open

github-actions Bot merged commit 695e851 into main Nov 9, 2026
18 checks passed

claude Bot mentioned this pull request Nov 9, 2026

feat(bench): MANIFEST.json names the mcp_server artifact behind the scores (#729) #947

Merged

8 of 9 tasks

Navigation Menu

Repository navigation

fix(tests): reranker cache-path tests compare paths, not strings (#692) - #952

fix(tests): reranker cache-path tests compare paths, not strings (#692)#952
github-actions[bot] merged 3 commits into
mainfrom
fix/692-reranker-cache-path-compare

cdeust commented Nov 9, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Nov 9, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 9, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 9, 2026

Uh oh!

claude Bot commented Nov 9, 2026 •

edited

Loading

Uh oh!

cdeust commented Nov 9, 2026

Uh oh!

claude Bot commented Nov 9, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cdeust commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of change

Test plan

Audit notes

Coding-standards compliance

Breaking changes

Reviewer checklist

Uh oh!

chatgpt-codex-connector Bot commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Nov 9, 2026

Review 1/3: PR #952

Uh oh!

claude Bot commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review 2/3: PR #952

Uh oh!

cdeust commented Nov 9, 2026

Uh oh!

claude Bot commented Nov 9, 2026

Review 2/3: PR #952

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cdeust commented Nov 9, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Nov 9, 2026 •

edited

Loading

claude Bot commented Nov 9, 2026 •

edited

Loading

claude Bot commented Nov 9, 2026 •

edited

Loading