fix(methodology): query_methodology lists hot memories under the domain remember writes (#721, slice 9) - #930

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

Review 1/3 of PR #930: no Blockers

• Read the diff, remember._resolve_domain and domain_mapping
• Ran five reviews: code quality, security, performance, tests, docs
• Posted the review: fix(methodology): query_methodology lists hot memories under the domain remember writes (#721, slice 9) #930 (comment)

The review found no Blockers and approves the PR. It lists three notes with no action needed before merge:

• Outside a known repository, reads and writes can still pick different domains: the handler detects the domain from cwd, project and first_message, but remember uses cwd only.
• With an empty cwd, the read and write scopes still differ.
• The CHANGELOG claim about detect_gaps is worded too broadly.

I did not run the tests, ruff or the craftsmanship gate here because they needed approval. The CI Pipeline run was still going when I posted.

Verdict: approve

Review 1/3 — PR #930 (slice 9 of #721)

I ran five reviews (code quality, security, performance, tests, docs). None found a Blocker. In a known repository, _memory_scope now reads the same canonical scope that remember._resolve_domain writes. The layers are respected (handler → shared) and the new function carries its source:. On the security side there's no injection (the domain is a bound parameter) and no new cross-project read. The registry is already cached (lru_cache), so the cost on the session-start path is negligible.

Notes, no action before merge

• Outside a known repository, read and write can still disagree: handler calls detect_domain with cwd+project+first_message (query_methodology.py:320-323), while remember calls it with cwd only (remember.py:87). The tests stub detect_domain with a constant, so they can't see this. A refinement PR could feed _memory_scope from detect_domain({"cwd": cwd}).
• With an empty cwd, _memory_scope returns resolve_hint(profile) but remember writes "". The docstring and CHANGELOG ("for the same cwd") claim more than the code does in that case. The domain_id is None case is also untested (store.domains[-1] would raise IndexError).
• The CHANGELOG says detect_gaps "reads only domains taken from the store". That holds for its store reads; _cognitive_style_gaps does use the caller's domain, but only for the profile detector.

Not verified by me: local tests, ruff and the craftsmanship gate (no permission to run them here). The CI Pipeline was still running when I posted this.

Verdict: approve

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

Review 2/3 of PR #930: no Blockers

• Checked review 1's Blockers: it found none.
• Found the commits pushed after 411c855: only fbba011 (fixup).
• Ran five reviews on fbba011: code quality, security, performance, tests, docs.
• Posted the review: fix(methodology): query_methodology lists hot memories under the domain remember writes (#721, slice 9) #930 (comment)

The fixup shortens _memory_scope to resolve_cwd(cwd) or resolve_hint(profile_domain). It behaves exactly like the old version: both functions already return "" on empty input, so the two mutants that survived were equivalent.

The review lists three notes, with no action needed before merge:

• A test can't tell "no domain read" from "read with """, so the directory fallback isn't checked.
• With an empty cwd, the docstring headline and the CHANGELOG still claim reads match remember's writes. Review 1 already raised this.
• The case with neither a cwd nor a profile is still untested.

I did not run the tests, ruff or the craftsmanship gate here.

Verdict: approve

CI autopilot : unit (et merge, qui en dépend) en échec sur 411c855, run 42347464717.

• Diagnostic : le gate de mutation a gardé deux mutants survivants de _memory_scope, if (cwd) or True et if (profile_domain) or True. Ils sont équivalents parce que les gardes étaient redondantes : resolve_cwd("") et resolve_domain("") renvoient déjà "". La cause est dans le code, pas dans les tests.
• Correctif fbba011 (fixup! de 411c855) : return resolve_cwd(cwd) or resolve_hint(profile_domain), la docstring décrit le cas sans cwd, et deux tests couvrent un cwd vide et un profil absent. Reproduit en local avec scripts/mutation_check.sh sur la même sélection de tests : 2 survivants avant, 0 après (rc=0). Les 7 fichiers de tests de la sélection : 64 passed ; ruff et le gate craftsmanship : OK.
• Non vérifié : le reste de la CI (regression, e2e, environment, install ont été sautés après l'échec de unit), la suite PostgreSQL en local.

Generated by Claude Code

Review 2/3 — PR #930 (slice 9 of #721)

Review 1 found no Blockers. This review covers only fbba011 (fixup), the one commit pushed after 411c855. I ran five reviews (code quality, security, performance, tests, docs) and none found a Blocker.

_memory_scope is now resolve_cwd(cwd) or resolve_hint(profile_domain), and it behaves exactly like the old guarded version for every input. Both functions already return "" on empty input before they call git or build the registry (domain_mapping.py:503-504, :407-408). That makes the two surviving mutants equivalent, so dropping the guards is the right fix. The fixup does not widen what gets read and adds no cost. The new IndexError guard in _read_scope closes the review-1 note about domain_id is None.

Notes, no action before merge

• tests_py/handlers/test_query_methodology_domain_scope.py:77: _read_scope returns "" both when get_memories_for_domain was never called and when it was called with "". So test_no_profile_…[outside] can't confirm that _get_hot_memories actually falls back to the directory (query_methodology.py:137). Recording which store method was called would fix that.
• query_methodology.py:160: the docstring headline still says "the domain remember writes". With an empty cwd, remember._resolve_domain writes "" (remember.py:90), while _memory_scope reads resolve_hint(profile). The new sentence describes the read correctly, but the headline and the CHANGELOG ("for the same cwd") should name this exception. Review 1 already raised this; it isn't new.
• No cwd plus no profile (the global get_hot_memories branch) is still untested.

Not verified by me: local tests, ruff and the craftsmanship gate.

Verdict: approve