Skip to content

fix(profiles): a rescan never lowers a domain's accumulated session evidence - #697

Merged
cdeust merged 4 commits into
mainfrom
fix/rebuild-profiles-keep-accumulated
Oct 10, 2026
Merged

cdeust merged 4 commits into
mainfrom
fix/rebuild-profiles-keep-accumulated

Conversation

@cdeust

@cdeust cdeust commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

rebuild_profiles no longer lowers a domain's accumulated session evidence: a domain whose scan sees fewer sessions than the stored profile records is kept and reported, and a deliberate replacement needs the explicit parameter replace_accumulated_profiles.

Symptom

Measured on the owner's machine on 2026-10-10: a forced rebuild turned the cortex domain from 32 recorded sessions (an April backup of the same key) into 8, the number of transcripts still on disk. Old transcripts are deleted by default retention, so the disk is a sliding window while the profile accumulates one session per record_session_end. The tool description said "rebuild from scratch" and force only bypasses the one-hour freshness check, so nothing told a caller that it replaces.

Root cause

mcp_server/core/profile_assembler.py, build_domain_profiles, old line 295: profiles["domains"][domain_id] = _build_single_domain(domain_id, data) replaced the stored profile for every domain with at least one transcript, with sessionCount = len(convs) of the scan.

Fix

  • New pure module mcp_server/core/profile_rebuild_policy.py decides per scanned domain: created (nothing stored), rebuilt (scan sees at least as many sessions as stored), kept (scan sees fewer, stored profile untouched), replaced (scan sees fewer and replace_accumulated_profiles=true).
  • A kept domain's in-memory profile is not written to: bridges, blind spots, feature activations and persona vector are not recomputed. Its scanned sessions still feed the shared feature dictionary and the persistence statistics.
  • The handler result gains domainOutcomes: one entry per scanned domain with domain, action, storedSessions, scannedSessions, resultingSessions. Domains with no transcripts on disk are untouched and not listed.
  • A stored sessionCount that is present but not a non-negative int stops the rebuild with ValueError; it is not read as 0, which would let the scan overwrite the profile.
  • The tool description and schema say that force only bypasses the freshness check and never replaces; replace_accumulated_profiles is marked destructive. The parameter is also in validation/schemas.py and in the registered wrapper (tool_registry_core.py); the full suite caught the missing wrapper argument (test_tool_schema_parity) on the first run.
  • record_session_end tool description no longer calls rebuild_profiles a from-scratch rescan.
  • profile_assembler.py was over the 300-line cap; helpers moved to profile_domain_stats.py and profile_domain_grouping.py, and its baseline entry is pruned. The existing handler fixture moved to tests_py/handlers/conftest.py so both test files share it.

The two checks the issue asks for

  • _compute_global_style sums sessionCount over every domain in the profile set, scanned or not (the 332 on the owner's index.json is the all-domain total after the rebuild). It recomputes from the domains, so it cannot double count a replaced profile. After this change a kept domain contributes its stored count and a rebuilt one the scan count. It is only recomputed by a rebuild; record_session_end does not update globalStyle (unchanged, pre-existing).
  • record_session_end does not share the bug: _update_profile calls apply_session_update (old_count + 1 and an EMA step) and save_profile for one domain; it never replaces a profile.

Precision on kept domains and out of scope

save_profiles rewrites every domain file of the store with its content and bumps updatedAt, so a kept domain's file is rewritten with identical content; only its in-memory profile is guaranteed untouched. The whole-store read-modify-write race with a concurrent record_session_end exists on main too and is out of scope here; it should be a follow-up issue (owner to file).

Decision record

ADR-1099 is in this PR (wiki page, docs mirror, manifest); the policy module cites it. rebuild_profiles now carries the DESTRUCTIVE tool annotation from _tool_meta.py because replace_accumulated_profiles can destroy accumulated state. A new test pins that a kept domain's connectionBridges are not recomputed; with the guard removed it fails (raw output: assert ['fresh-bridge'] == ['stored']).

_check_skip (issue 685)

Not touched. The fix does not need it; the freshness check keeps its current behaviour.

Failing before, passing after

Before, on main with the new tests (raw output in the first lines):

E   assert 8 == 32
tests_py/handlers/test_rebuild_profiles_accumulation.py:94: assert 8 == 32
...
18 failed, 11 passed in 73.03s (0:01:13)

After, on the final head 8d6cf03, full suite pytest -q -p no:cacheprovider (raw output in /Users/cdeust/.claude/fleet/work/Cortex/A2-full-pytest.txt):

9605 passed, 19 skipped, 2 warnings, 373 subtests passed in 463.19s (0:07:43)
exit=0

Other gates on 8d6cf03, all pass: uvx ruff@0.16.6 check, uvx ruff@0.16.6 format --check, python scripts/check_craftsmanship.py --base origin/main (prints OK), python scripts/check_project_wiki.py, python scripts/check_no_deps_invariant.py, pyright mcp_server/ (0 errors, environment from uv.lock with the typecheck group and the otel extra, arm64 Python).

Registry value flow (review round 2): TestRegisteredToolPath calls rebuild_profiles through mcp.call_tool and reads the stored profile back. Mutant that forwards a hard-coded False in tool_registry_core.py: test_true_reaches_the_handler_and_replaces fails (1 failed, 25 passed, A-mutant-registry-false.txt). Mutant that forwards a hard-coded True: test_force_alone_keeps_the_accumulated_profile fails (A-mutant-registry-true.txt). Both pass on the real wrapper.

Completion Ledger

Branch or error path Test
kept (scan 8, stored 32), stored profile unchanged, reported test_rebuild_profiles_accumulation.py TestRescanSeesFewerSessions::test_stored_profile_is_kept, test_kept_profile_derived_fields_are_not_rewritten; test_profile_rebuild_policy.py test_kept_profile_is_untouched_in_every_field
replaced with before and after counts TestRescanSeesFewerSessions::test_explicit_replace_replaces_and_reports_before_and_after; test_replace_builds_from_the_scan
force alone never replaces TestToolContract::test_force_alone_never_replaces
rebuilt (scan more, scan equal) TestRescanSeesAtLeastAsManySessions::test_domain_is_rebuilt[3], [8]
created (nothing stored) test_new_domain_is_created
domain with no transcripts untouched, not reported TestDomainsTheScanDoesNotSee::test_domain_without_conversations_is_untouched_and_unreported; test_no_scanned_conversations_leaves_stored_profiles_and_reports_nothing
target domain limits report and writes test_target_domain_limits_what_is_reported_and_touched
malformed stored sessionCount fails hard TestMalformedStoredCount (four bad types), test_malformed_count_raises_naming_the_domain, test_malformed_stored_count_aborts_the_build
missing sessionCount counts as 0 test_missing_session_count_means_no_recorded_evidence, test_absent_or_null_count_is_zero
global style with a kept domain and after replace test_global_style_counts_the_kept_sessions_once, test_global_style_weights_the_kept_domain_by_its_stored_count, test_global_style_after_replace_counts_the_scan
tool schema, description, validation schema TestToolContract, test_tool_schema_parity.py
value flow through the registered wrapper (true replaces, force alone keeps) TestRegisteredToolPath

All handler tests use a temporary directory for the scanner root and the methodology store; nothing reads or writes the real home directory.

Decision points for an ADR

(Registered as ADR-1099.)

  1. A rebuild never lowers accumulated evidence: kept when scanned < stored sessionCount, rebuilt when scanned >= stored.
  2. The explicit override is replace_accumulated_profiles; force keeps meaning only "bypass the freshness check".
  3. Known limit of the rule: it compares counts. A scan with at least as many sessions still replaces the accumulated moving-average state with a scan-derived one, as before.

Not verified

Windows. The load average on the machine was above 100 while the suite ran, so only the pass/fail result is meaningful, not the duration.

Fixes #686

🤖 Generated with Claude Code

https://claude.ai/code/session_01DXMXvHLm94B6pA9FkWWmzu

@cdeust

cdeust commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

ZETETIC-REVIEW: REQUEST_CHANGES
f64ab9a

Scope: code, tests, text. Worktree .claude/worktrees/review-697 (detached at the head). Load was 2-3 during the runs; only targeted tests.

BLOCKING

  1. ADR-1099 is not in the PR. git diff --stat origin/main...f64ab9a lists 16 files, none under docs/adr or wiki/adr; origin/main has no ADR-1099 (latest is 1098). The policy module docstring (mcp_server/core/profile_rebuild_policy.py:14) still says "a new decision, pending registration as a wiki ADR", and no pointer cites ADR-1099. The lead's patch (697-adr-1099.patch) has to be applied and the docstring pointer changed to cite ADR-1099.
  2. A surviving mutant on a stated guarantee. In mcp_server/core/profile_assembler.py (_attach_bridges_and_blind_spots) the guard if domain_id in profiles["domains"] and domain_id not in kept: for connectionBridges can be removed (kept domain gets bridges rewritten) and tests_py/handlers/test_rebuild_profiles_accumulation.py plus tests_py/core/test_profile_rebuild_policy.py all still pass (40 passed). The PR body promises a kept domain's bridges are not recomputed; no test asserts it. Add a fixture where find_bridges yields a bridge for the kept domain and assert the stored connectionBridges is unchanged.

NON-BLOCKING

  • "A kept domain is not written to at all" is true of the in-memory profile; save_profiles (mcp_server/infrastructure/profile_store.py:165) still rewrites every domain file (kept ones with identical content) and bumps updatedAt. Wording in the PR/docstrings is a little strong; it is a read-modify-write of the whole store, so a record_session_end hook write landing between load_profiles and save_profiles in the handler is lost (race exists on main already, not introduced here).
  • The shared featureDictionary and persistentFeatures are relearned from the scan, so a kept domain's stored featureActivations (keyed by feature label) may refer to an older dictionary. Same already holds for unscanned domains on main; stated limit in the PR (count comparison, scan >= stored still replaces the EMA state) is acceptable: a scan seeing at least as many sessions covers every recorded session, so count evidence is not lowered.
  • Tool-level annotation of rebuild_profiles is still IDEMPOTENT_WRITE; "destructive" is in the parameter description only. I did not verify whether a destructiveHint is expected by any parity test (tests passed).

VERIFIED (command/source)

  • Q1: policy read in full: created / rebuilt (scanned >= stored) / replaced / kept; stored count None counts 0; bool, float, str, negative raise ValueError (policy lines ~53-66); raised inside _rebuild_or_keep, before save_profiles (handler line 139), so nothing is saved on error; handler has no catch. Merged-domain split by project path: new id is "created", old id unscanned and untouched, no surprise. target_domain filters at group_conversations_by_domain, so reporting and writing are both limited (test_target_domain_limits_what_is_reported_and_touched passes). globalStyle is recomputed from the domains (compute_global_style), counts each domain once. Kept domains: bridges, blind spots, featureActivations, personaVector are guarded by kept (blind-spot and activation guards are covered: removing either fails a test; bridge guard is not, see 2). Mutating the shared dict in place: the only writes to a kept domain are those guarded paths.
  • Q3: schema (handler), validation/schemas.py, tool_registry_core wrapper in sync; parameter optional default False; force=true callers (skills/cortex-setup-project/SKILL.md) get the safe behaviour. tests_py/handlers/test_tool_schema_parity.py passes. Docs: docs/api-reference.md and docs/data-flow.md updated. Grepped README, docs, skills, commands: no remaining "rebuild_profiles ... from scratch" statement; docs/mcp-tools.md "Full rescan of session data" is still true.
  • Q4: git grep: only rebuild_profiles.py calls build_domain_profiles and save_profiles; record_session_end.py:286 and hooks/session_lifecycle.py:376 (SessionEnd hook, same apply_session_update + save_profile of one domain, guarded by sessionEndEvents) increment, not replace. Readers of sessionCount (list_domains, query_methodology, explore_features, context_generator) only read. No sibling shares the defect. build_domain_profiles now returns ProfileBuild instead of dict: the only non-test caller is the handler; profile_builder.py's "re-exports build_domain_profiles" comment is stale but it has no re-export (grep).
  • Q5: ran tests_py/handlers/test_rebuild_profiles_accumulation.py, tests_py/core/test_profile_rebuild_policy.py, tests_py/handlers/test_rebuild_profiles.py, tests_py/core/test_profile_builder.py, tests_py/infrastructure/test_profile_store.py, tests_py/handlers/test_tool_schema_parity.py: 78 passed. With the base profile_assembler.py and rebuild_profiles.py restored and the new test kept: 17 failed, 1 passed. Pre-existing touched tests: test_rebuild_profiles.py 33 def/assert count both sides, test_profile_builder.py 61 both sides; changes are mechanical (_build wrapper returning .profiles, ProfileBuild mock return values, fixture isolation moved to hermetic_claude_dirs); meaning unchanged. Hand mutations (each run against accumulation + policy tests): >= to > fails; accept bool fails; drop replace branch fails; replace always fails; kept writes the profile fails; allow negative fails; resultingSessions for kept = scanned fails; remove blind-spot kept guard fails; remove activation kept guard fails; remove bridge kept guard SURVIVES (finding 2). Tests use tmp dirs through hermetic_claude_dirs; I saw no sleeps.
  • Q6: pure move of the five helpers into profile_domain_stats.py / profile_domain_grouping.py (renamed without underscore; diff read); core imports only core and shared. Sizes: profile_assembler.py 263, stats 77, grouping 49, policy 107, handler 148, new tests 214/161. python scripts/check_craftsmanship.py --base origin/main: OK. uvx ruff@0.16.6 check: all passed; format --check: 3541 files formatted; scripts/check_no_deps_invariant.py: clean. Baseline entry for profile_assembler.py removal is consistent with the gate OK.
  • Q7: git merge-tree --write-tree origin/main refs/review/697 returns a tree with no conflict against current main (fix(hooks): write hook output as UTF-8 on every platform and entry point #695, fix(benchmarks): run bench_regression.sh under macOS bash 3.2 and sweep the shell scripts #696 included). ADR number 1099 is free on origin/main. PR body has the symptom, root cause, fix list; I read the first 2500 chars only (Fixes rebuild_profiles replaces accumulated domain profiles with a smaller scan #686 line and ledger/unverified section not seen by me).

NOT VERIFIED

  • Full suite (author: 9460 passed); CI (not looked at); how safe_handler renders the ValueError to an MCP caller (no catch in the handler, I did not read tool_error_handler.py); PR body tail (ledger rows, Fixes rebuild_profiles replaces accumulated domain profiles with a smaller scan #686); ledger spot-check of eight rows was done through the test names and mutations above, not row by row against the PR text.

cdeust and others added 3 commits October 10, 2026 08:37
…vidence

rebuild_profiles replaced every scanned domain's stored profile with one
computed from the transcripts currently on disk. Old transcripts are deleted
by the host's default retention, so that is a sliding window, while the
stored profile accumulates one session per record_session_end. A forced
rebuild turned the cortex domain from 32 recorded sessions into 8 on the
owner's machine.

A domain whose scan sees fewer sessions than the stored profile records is
now kept untouched and reported; a scan that sees at least as many rebuilds
as before. replace_accumulated_profiles is the explicit, destructive
override (force only bypasses the freshness check). The result carries
domainOutcomes per scanned domain with stored, scanned and resulting counts.
A non-integer stored sessionCount stops the rebuild instead of reading as 0.

profile_assembler.py shrinks under the 300-line cap (helpers moved to
profile_domain_stats.py and profile_domain_grouping.py) and its baseline
entry is pruned. The record_session_end tool description no longer calls
rebuild_profiles a from-scratch rescan.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXMXvHLm94B6pA9FkWWmzu
Signed-off-by: cdeust <cdeust@icloud.com>
…ted session evidence

Registers the decision behind the rebuild_profiles change in this branch.
It amends ADR-0429 and ADR-0227.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXMXvHLm94B6pA9FkWWmzu
Signed-off-by: cdeust <cdeust@icloud.com>
…pin kept bridges

The policy module cites ADR-1099. rebuild_profiles now carries the
DESTRUCTIVE tool annotation because replace_accumulated_profiles can destroy
accumulated state. A new test pins that a kept domain's connectionBridges are
not recomputed and fails when the guard is removed. The profile_builder
docstring no longer claims a re-export that does not exist.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXMXvHLm94B6pA9FkWWmzu
Signed-off-by: cdeust <cdeust@icloud.com>
@cdeust
cdeust force-pushed the fix/rebuild-profiles-keep-accumulated branch from f64ab9a to 2b243a9 Compare October 10, 2026 06:47
…e registered tool

The accumulation tests call the handler directly and the schema parity test
only compares parameter sets, so a wrapper that kept the parameter and
forwarded a constant passed the whole handlers directory. The wiring bug
found earlier in this change was exactly that class.

Two tests now call rebuild_profiles through mcp.call_tool and read the stored
profile back: true replaces the accumulated profile, force=true alone keeps
it. Forwarding a hard-coded False fails the first; forwarding a hard-coded
True fails the second.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXMXvHLm94B6pA9FkWWmzu
Signed-off-by: cdeust <cdeust@icloud.com>
@cdeust

cdeust commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

ZETETIC-REVIEW: APPROVE
8d6cf03

Scope: code, tests, text. Delta 2b243a9..8d6cf03. Worktree review-697c (detached, arm64 python, removed afterwards). uptime 2.1 before runs; targeted tests only; ps shows no pytest left.

BLOCKING: none. The earlier required finding (no test through the registered wrapper) is closed.

VERIFIED

  1. Delta: git diff 2b243a99 8d6cf03e --numstat: one file, tests_py/handlers/test_rebuild_profiles_accumulation.py, +32/-0 (imports MCPServer and tool_registry_core, class TestRegisteredToolPath). Main is an ancestor of the head. No source change.
  2. Test reading: it builds a fresh MCPServer, calls tool_registry_core._register_rebuild_profiles(mcp) (the same function register_all calls at tool_registry_core.py:44, so the same wrapper object, not a re-implementation), then asyncio.run(mcp.call_tool("rebuild_profiles", args)). It reads the stored profile with load_profile and asserts content: sessionCount 8 and accumulatedMarker absent for replace, sessionCount 32 and marker present for force alone. It uses the existing window fixture on hermetic_claude_dirs (temp dirs, not real ~/.claude). Not a returned flag.
  3. Mutants, each restored by git checkout: (a) forward False in tool_registry_core.py: 1 failed (test_true_reaches_the_handler_and_replaces), 19 passed. (b) forward True: 1 failed (test_force_alone_keeps_the_accumulated_profile). (c) delete the parameter line from validation/schemas.py: test_validation_schema_accepts_the_replace_parameter fails (existing test, KeyError) and test_tool_schema_parity plus the rest pass 25; the new class does not catch (c) itself, the existing test does.
  4. Targeted run at the head: accumulation, test_profile_rebuild_policy, test_tool_schema_parity, test_tool_profiles, handlers/test_registry: 66 passed. Unchanged pre-existing files: test_profile_builder.py 18 tests / 43 asserts base and head; test_rebuild_profiles.py 11 / 22 base and head. python scripts/check_craftsmanship.py --base origin/main: "Craftsmanship gate: OK". uvx ruff@0.16.6 check .: all passed; format --check .: 3552 already formatted.
  5. PR body (gh pr view 697): headRefOid is the head above; contains Fixes #686, mentions TestRegisteredToolPath and the in-memory precision.

NON-BLOCKING (nit)

  • The test calls the private _register_rebuild_profiles, not register_all; same wrapper, but a rename of that private function breaks the test rather than the product. Wording-level, no action required.
  • ADR-1099 sentence "not written at all" left as is by the author (previous reviews raised it; PR body states the in-memory scope). Wording only.

NOT VERIFIED
Full suite (author reports 9605 passed, 19 skipped), pyright, CI (not looked at), wiki and deps checks (not re-run in this delta, no docs or imports in non-test code changed), disk_hygiene registration (register-worktree answered "protected: not an available unlocked linked branch worktree"; I removed the worktree with git directly).

@cdeust
cdeust merged commit 02c3cf0 into main Oct 10, 2026
28 checks passed
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.

rebuild_profiles replaces accumulated domain profiles with a smaller scan

1 participant