fix(scope): the SQL project predicate normalizes Windows separators like the Python rule (#692) by cdeust · Pull Request #959 · cdeust/Cortex

cdeust · 2026-11-10T01:37:18Z

Summary

Tranche classe 9 « file priming aliases » de #692. Refs #692 (le suivi Windows reste ouvert pour les autres tranches).

Cause racine. project_ancestors() normalise la racine de session (\ → /, séparateur final retiré) et memory_matches_project() normalise aussi le directory_context stocké (ADR-1080). Les prédicats SQL, eux, comparaient la liste normalisée à la colonne brute. Une ligne dont le directory_context contient des \ ou finit par un séparateur passait la règle Python mais jamais la requête SQL. Dans la mesure Windows (run 38008915302, job 114084097957), le priming laissait donc les lignes à heat 0.5 (test_file_priming_aliases ×4, test_file_memory_priming::test_alias_project_read_preserves_stored_scope ×2, assert 0.5 == 0.6).

Correctif. pg_scope_clause.normalized_directory_column() est maintenant la seule expression RTRIM(REPLACE(directory_context, '\', '/'), '/'), valide en PostgreSQL et en SQLite. Les deux directory_scope_clause l'utilisent, ainsi que les 6 requêtes PostgreSQL brutes des hooks, qui en avaient chacune une copie inline : session_start_fetch ×3, auto_recall, agent_briefing_query ×2 et agent_briefing_role. Aucun index ne couvre directory_context, donc envelopper la colonne ne retire aucun usage d'index. Pour un chemin POSIX déjà normalisé, l'expression ne change rien.

Type of change

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

Test plan

Nouveau tests_py/infrastructure/test_scope_clause_separators.py. Il écrit l'orthographe Windows sur n'importe quelle plateforme, donc la CI Linux couvre le défaut :
- priming et get_hot_memories acceptent C:\work\repo, C:\work\repo\, C:/work/repo et l'ancêtre C:\work, et rejettent C:\work\other ;
- un test statique vérifie qu'aucune requête de mcp_server/ ne compare la colonne brute (directory_context = ANY / IN (), pour éviter que les copies réapparaissent.
- Sur main sans le correctif : 6 failed, 2 passed (les 6 cas à \ ou à séparateur final). Avec le correctif : 9 passed.
pytest tests_py/infrastructure tests_py/hooks tests_py/shared : 3083 passed, 195 skipped, 6 failed. Les 6 échecs sont identiques sur origin/main dans le même environnement : test_embedding_engine ×3 et test_vulnerable_dependency_paths (torch absent, son index de téléchargement est bloqué par le réseau), plus test_entry_auto_dispatch ×2.
ruff check et ruff format --check sur les fichiers modifiés : OK. python scripts/check_craftsmanship.py : OK.

Audit notes

Non vérifié localement : aucun PostgreSQL avec pgvector ici, donc les requêtes PG des hooks et directory_scope_clause côté PG ne sont exercées que par la CI. Le littéral '\' suppose standard_conforming_strings = on (défaut PostgreSQL depuis 9.1).
Aucun run Windows ; il faut une nouvelle mesure Windows pour confirmer que les 6 tests passent.
benchmarks/reproduce.sh n'a pas été lancé (il faut PostgreSQL et les modèles). Les chemins des benchmarks sont POSIX, et l'expression ne les modifie pas.
Changement de comportement voulu : une ligne stockée avec un séparateur final (/repo/) entre maintenant dans la portée SQL de /repo, comme la règle Python l'acceptait déjà.

Coding-standards compliance

§2.2 Layer dependency direction preserved (les hooks importaient déjà infrastructure.sqlite_scope_clause).
§8 / gate craftsmanship OK.

Reviewer checklist

CHANGELOG.md updated under the appropriate section.
No secrets / credentials / PII in the diff.

Generated by Claude Code

…ike the Python rule (#692) project_ancestors() slash-normalizes the session root and memory_matches_project() normalizes the stored directory_context too (ADR-1080), but every SQL scope predicate compared the ancestor list with the raw column. A row whose directory_context uses backslashes or ends with a separator matched in Python and never in SQL: on the Windows measurement (run 38008915302, job 114084097957) file priming left aliased rows at heat 0.5 (test_file_priming_aliases x4, test_file_memory_priming x2). pg_scope_clause.normalized_directory_column() is now the one expression (REPLACE then RTRIM, valid in PostgreSQL and SQLite) used by both backends' scope clauses and by the session-start, auto-recall and agent-briefing raw PostgreSQL queries, which carried inline copies of the predicate. Refs #692 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYudG4Cbi77EfobNUhL

chatgpt-codex-connector · 2026-11-10T01:37:26Z

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-10T03:58:26.995035Z	`09bd34f`	New commits
🔒 Security Review	✅ Completed 2026-11-10T04:01:06.168009Z	`09bd34f`	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-10T02:19:07Z

Claude finished @cdeust's task in 2m 29s —— View job

I'll analyze this and get back to you.

claude · 2026-11-10T02:21:41Z

Review 1/3: PR #959

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

What they traced:

Normalization matches Python. RTRIM(REPLACE(col, '\', '/'), '/') behaves like project_scope._normalize_path in every case traced: repeated trailing separators, / and C:\ roots, NULL, empty, and case. project_ancestors never returns "" or a bare drive, so no new match crosses into another project.
Every ancestor-scope site is covered. All of them now go through pg_scope_predicate or directory_scope_clause. Bound-parameter count and order are unchanged in every hook query.
No user input reaches the SQL text. column_prefix is always a hard-coded literal, so the # noqa: S608 notes are accurate.

Not run in this review: ruff, the craftsmanship gate and pytest. Running them needed approval.

Notes, no action before merge

mcp_server/infrastructure/pg_scope_clause.py:24: "No index covers directory_context" is only true for PostgreSQL. SQLite's idx_memories_heat_tie and idx_memories_created_tie (sqlite_schema.py:374-379) have it as their last column. No index seek is lost, because the column is never a seek key, but the sentence should be narrowed.
Some exact-directory filters still compare the raw column: sqlite_store_recall_signals.py:218, pg_schema.py:1028 (p_directory), and get_memories_for_directory in pg_store_queries.py:47 and sqlite_store_queries.py:58. They are outside ADR-1080 ancestor scope, so they could be a later Windows: full-suite debt of 194 failing tests (measured 2026-10-10), fix order by root cause #692 tranche.
The test_no_query_compares_the_raw_directory_column regex only catches = ANY and IN (. Its docstring says "cannot run here without pgvector", but the CI "unit" job has pgvector. None of the hook PG tests uses a Windows path yet.

Verdict: approve

cdeust · 2026-11-10T02:22:05Z

Réponse aux notes de la revue 1/3 (aucune n'est bloquante, donc aucun push pour elles seules) :

pg_scope_clause.py:24 : c'est exact, et c'est une imprécision de ma part. Sous SQLite, idx_memories_heat_tie et idx_memories_created_tie portent directory_context en dernière colonne (sqlite_schema.py:374-379). Ce sont des index couvrants, et SQLite calcule l'expression RTRIM(REPLACE(…)) depuis la colonne indexée : l'index reste donc utilisable. La phrase sera restreinte à « aucun index ne prend directory_context comme clé de recherche » au prochain push de code.
Filtres par répertoire exact (sqlite_store_recall_signals.py:218, pg_schema.py:1028, get_memories_for_directory) : ils sont hors du périmètre ancêtres d'ADR-1080, donc je ne les ajoute pas ici. Je les note comme candidats pour une tranche suivante de Windows: full-suite debt of 194 failing tests (measured 2026-10-10), fix order by root cause #692, à confirmer par la prochaine mesure Windows.
Docstring du test statique : « cannot run here » voulait dire « localement ». Je la corrigerai au même push, et j'y préciserai que la regex ne couvre que les formes = ANY et IN (.

Generated by Claude Code

…692) The unit check's mutation step (ADR-1122, run 42737080540, job 129576254714) kept 7 survivors on the lines 3ac8699 changed: - agent_briefing_query._fetch_agent_context 12, 13 and 15 drop the pass-1 rows. No test ever had pass 1 return a row: the existing PG scope test only reaches team decisions through pass 2. test_pg_briefing_passes_scope_a_ windows_directory makes both passes and _pg_role_rows return rows stored under C:\work\orchid (one with a trailing separator), and exclude a sibling directory. - agent_briefing_role._pg_role_rows 8, 9, 23 and 26 only change the case of SQL keywords and of the unquoted alias. The query moves to the module constant _PG_ROLE_SQL, like session_start_fetch's queries, so it is no longer mutated as function code. Also narrows two docstrings from review 1/3: SQLite has covering indexes ending with directory_context, and the static guard covers only the "= ANY(" and "IN (" forms. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYudG4Cbi77EfobNUhL

cdeust · 2026-11-10T02:52:30Z

CI autopilot : unit rouge sur 3ac8699, correctif 903cd34 poussé.

Les 12 354 tests passent (run 42737080540, job 129576254714, image ubuntu-24.04). L'échec vient de l'étape de mutation sur les lignes modifiées (ADR-1122), qui garde 7 survivants.

Vrai trou de test : dans agent_briefing_query._fetch_agent_context, les mutants 12, 13 et 15 suppriment les lignes de la passe 1, et aucun test ne le voit. Le test PG existant n'atteint les décisions d'équipe que par la passe 2 ; la passe 1 n'y renvoie jamais de ligne.
- Le nouveau test_pg_briefing_passes_scope_a_windows_directory stocke des lignes sous C:\work\orchid, dont une avec séparateur final. Il fait renvoyer ces lignes à la passe 1, à la passe 2 et à _pg_role_rows, et exclut C:\work\other.
Mutants équivalents : dans agent_briefing_role._pg_role_rows, les mutants 8, 9, 23 et 26 ne changent que la casse des mots-clés SQL et de l'alias non quoté. La requête passe dans la constante de module _PG_ROLE_SQL, comme celles de session_start_fetch, et n'est plus mutée comme du code de fonction.
Le même commit restreint les deux docstrings signalées par la revue 1/3.

Non vérifié :

Le nouveau test PG n'a pas tourné localement. Le pgvector d'apt est en 0.6 et le schéma exige au moins 0.8.0 ; la compilation depuis les sources m'a été refusée.
Je n'ai pas non plus pu rejouer le gate de mutation localement, faute de PostgreSQL. Seule la CI en donnera le reçu.
En local, les tests SQLite des fichiers touchés passent (21 passed, 6 skipped PG). ruff et le gate craftsmanship sont OK.

Generated by Claude Code

claude · 2026-11-10T03:37:15Z

Claude finished @cdeust's task in 2m 38s —— View job

I'll analyze this and get back to you.

claude · 2026-11-10T03:39:56Z

Review 2/3: PR #959

Review 1/3 found no Blockers, so this pass covers only the commit pushed after 3ac8699: 903cd34. Five reviewers ran on it (code quality, security, performance, test coverage, documentation). None found a Blocker.

What they traced:

_PG_ROLE_SQL is correct. The module constant is built once at import, from literals and pg_scope_predicate("m.") with a fixed prefix, so no input reaches the SQL text. Its four %s line up with (agent, _MIN_HEAT, project_ancestors(project_root), remaining). It has a # source: line, and the gate limits are met.
The new PG test checks the fix. remember stores directory as given (remember_helpers.py:520), so the backslash rows really reach the database. The priors are not team decisions and the dba row is one. The pass 1 query plainto_tsquery(english, ledger) matches the prior. Without the normalization, the asserts at lines 116, 117 and 120 fail, which kills the pass-1 survivors named in the commit message.
The index docstring is now accurate. No PG index touches directory_context. The SQLite tie indexes only end with it, so it is never a seek prefix.

Not run in this review: ruff, the craftsmanship gate and pytest (no approved shell), and the PG-only test (it skips on SQLite).

Notes, no action before merge

tests_py/hooks/test_team_project_scope.py:101: "Decision: …" is classified as a team decision only through the Spanish cue \bdecisi[oó]n\w* (content_cues.py:34). Tightening that pattern would break this test for a reason that has nothing to do with scope.
tests_py/infrastructure/test_scope_clause_separators.py:62-63: the docstring says the Windows-path test runs the PostgreSQL queries of the hooks. It runs only _fetch_agent_context and _pg_role_rows, not session_start_fetch or auto_recall.
tests_py/hooks/test_team_project_scope.py:120: adding decision not in role_ids would also pin the NOT COALESCE(m.is_team_decision, FALSE) exclusion, at no extra cost.

View job run

Verdict: approve

cdeust · 2026-11-10T03:40:14Z

Réponse aux notes de la revue 2/3. Aucune n'est bloquante ; je ne pousse pas pour elles seules, car un push relance la CI et une revue.

test_team_project_scope.py:101 : c'est vrai, la classification « team decision » passe par l'indice \bdecisi[oó]n\w*. Le test existant test_pg_team_readers_scope_both_agent_passes repose sur la même formulation. Si cet indice change, les deux tests le signaleront ensemble, comme une rupture de classification et non de portée.
Docstring du test statique et decision not in role_ids : ces deux points sont justes. Je les reprendrai au prochain push de code, s'il y en a un sur cette PR ; sinon dans la prochaine tranche de Windows: full-suite debt of 194 failing tests (measured 2026-10-10), fix order by root cause #692.

Generated by Claude Code

…cess (#692) The unit check's mutation step (run 42745672842, job 129607262143) passed the briefing functions but kept two gaps on 903cd34: - auto_recall._recall_memories 4, 5 and 7 drop its query rows. test_auto_recall_scope runs the hook as a subprocess, which mutmut's trampoline does not reach. The Windows-directory PG test now calls _recall_memories in process and requires the scoped rows back. - pg_scope_clause and sqlite_scope_clause were untested: no test file imported them. test_scope_clause_separators now imports both and pins each builder's output; locally scripts/mutation_check.sh on the two modules leaves no survivor. Also from review 2/3: the role-prior query must exclude the team decision, and the static guard's docstring names the queries the PG test runs. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYudG4Cbi77EfobNUhL

cdeust · 2026-11-10T03:57:34Z

CI autopilot : unit rouge sur 903cd34, correctif 09bd34f poussé.

Les 12 355 tests passent (run 42745672842, job 129607262143). Les mutants de agent_briefing_query et de agent_briefing_role sont maintenant tués. Il reste deux trous.

auto_recall._recall_memories : les mutants 4, 5 et 7 suppriment les lignes de la requête sans qu'aucun test ne le voie. test_auto_recall_scope lance le hook en sous-processus, et le trampoline de mutmut ne s'y applique pas. Le test PG sur répertoire Windows appelle désormais _recall_memories dans le processus et exige les lignes de la portée.
pg_scope_clause et sqlite_scope_clause : ils étaient « untested », parce qu'aucun fichier de test ne les importait. test_scope_clause_separators les importe maintenant et fige la sortie de chaque constructeur. En local, scripts/mutation_check.sh sur ces deux modules ne laisse aucun survivant.
Le commit reprend aussi les deux points de la revue 2/3 : decision not in role_ids, et la docstring du garde statique.

Non vérifié : le test PG et la mutation d'auto_recall n'ont pas pu tourner localement (pas de pgvector ≥ 0.8). Seule la CI en donnera le reçu.

Generated by Claude Code

claude · 2026-11-10T04:45:42Z

Claude finished @cdeust's task in 2m 44s —— View job

Review 3/3: PR #959

Confirm review 2/3 left no Blockers; scope is the one commit after 903cd34: 09bd34f, which changes tests only
Five reviewers (code quality, security, performance, test coverage, documentation): no Blocker
Post the verdict: review comment

The whole PR is accepted. All three commits and all 10 files are clear, and no Blocker is open. The review comment lists three Notes that need no action before merge. The main one: in tests_py/hooks/test_team_project_scope.py:123, the check decision not in role_ids passes because of the agent filter alone, so it does not pin the is_team_decision exclusion.

Not run in this review: ruff, the craftsmanship gate and pytest.

Verdict: approve

claude · 2026-11-10T04:48:26Z

Review 3/3: PR #959

Review 2/3 found no Blockers, so this pass covers only the commit pushed after 903cd34: 09bd34f, which changes tests only. Five reviewers ran on it (code quality, security, performance, test coverage, documentation). None found a Blocker.

What they traced:

The new _recall_memories assertions hold on PostgreSQL.
- _write uses force=True, so the three rows stay separate.
- A new memory's heat (≥ 1.0) is above _MIN_HEAT (0.15).
- plainto_tsquery(english, ledger orchid) matches all three contents.
- C:\work\orchid and C:\work\orchid\ both normalize to C:/work/orchid, and C:\work\other does not.
- Two rows match, under LIMIT _MAX_MEMORIES + 2.
- The autouse _test_isolation fixture purges PG tables between tests (tests_py/conftest.py:282-284).
- The catch-all except returns [], which would fail the test loudly rather than pass it.
The shape tests match the builders exactly. That includes the backslash escaping in _NORMALIZED, the trailing spaces and the SQLite empty-list branch. Before this commit no test imported pg_scope_clause or sqlite_scope_clause (git grep 903cd34 -- tests_py is empty), as the commit message says.
Gate: _NORMALIZED is a string, so rule 4 (# source: on numeric constants) does not apply. The longest test is 36 lines.

Not run in this review: ruff, the craftsmanship gate and pytest. The PG-only test skips on SQLite.

Notes, no action before merge

tests_py/hooks/test_team_project_scope.py:123: decision not in role_ids holds through m.agent_context = %s alone, because the row's agent_topic is "dba". A mutant that drops NOT COALESCE(m.is_team_decision, FALSE) would survive. An in-scope "Decision: …" row with agent_topic="engineer" would pin it.
tests_py/hooks/test_team_project_scope.py:93: "so mutation testing reaches the queries" is true only for a mutation run on the PostgreSQL backend, because the test skips on SQLite.
tests_py/infrastructure/test_scope_clause_separators.py:89: the PG directory_scope_clause([]) case (global-only through an empty array) is not asserted; the SQLite one is.

Accepted: the whole PR, all three commits (3ac8699, 903cd34, 09bd34f) and all 10 files. No file or commit carries an open Blocker.

View job run

Verdict: approve

cdeust · 2026-11-10T04:48:46Z

Réponse aux notes de la revue 3/3 : aucune n'est bloquante. Je ne pousse rien pour elles : le budget de revue est épuisé (3/3, ADR-1119), et un push relancerait la CI pour des points de test seulement.

test_team_project_scope.py:123 : c'est exact, decision not in role_ids passe par le seul filtre agent_context. Pour figer l'exclusion is_team_decision, il faut une décision du même agent dans la portée.
test_team_project_scope.py:93 : la phrase vaut pour un passage de mutation sur PostgreSQL, ce que fait le job unit de la CI. Je la préciserai.
test_scope_clause_separators.py:89 : le cas PG directory_scope_clause([]) n'est pas encore figé.

Ces trois points sont notés pour la prochaine tranche de #692.

Generated by Claude Code

cdeust mentioned this pull request Nov 10, 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 f95ce15 into main Nov 10, 2026
18 checks passed

Navigation Menu

Repository navigation

fix(scope): the SQL project predicate normalizes Windows separators like the Python rule (#692) - #959

fix(scope): the SQL project predicate normalizes Windows separators like the Python rule (#692)#959
github-actions[bot] merged 3 commits into
mainfrom
fix/692-windows-scope-separators

cdeust commented Nov 10, 2026

Uh oh!

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

edited

Loading

Uh oh!

claude Bot commented Nov 10, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 10, 2026

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

claude Bot commented Nov 10, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 10, 2026

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

claude Bot commented Nov 10, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 10, 2026

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cdeust commented Nov 10, 2026

Summary

Type of change

Test plan

Audit notes

Coding-standards compliance

Reviewer checklist

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

claude Bot commented Nov 10, 2026

Review 1/3: PR #959

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

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

Uh oh!

Uh oh!

claude Bot commented Nov 10, 2026

Review 2/3: PR #959

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

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

Uh oh!

Review 3/3: PR #959

Uh oh!

claude Bot commented Nov 10, 2026

Review 3/3: PR #959

Uh oh!

cdeust commented Nov 10, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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

edited

Loading

claude Bot commented Nov 10, 2026 •

edited

Loading

claude Bot commented Nov 10, 2026 •

edited

Loading

claude Bot commented Nov 10, 2026 •

edited

Loading