Skip to content

fix(profiles): rebuild_profiles creates a domain from a Codex-only history (#726, slice 1) - #881

Merged
github-actions[bot] merged 3 commits into
mainfrom
fix/726-codex-profile-bootstrap
Nov 6, 2026
Merged

github-actions[bot] merged 3 commits into
mainfrom
fix/726-codex-profile-bootstrap

Conversation

@cdeust

@cdeust cdeust commented Nov 5, 2026

Copy link
Copy Markdown
Owner

Summary

First slice of #726: rebuild_profiles now creates a domain from a Codex-only history. It walked ~/.claude/projects only, and scanner_parse.extract_message_stats reads top-level user/assistant records, so a Codex rollout (session_meta, response_item, event_msg) parsed to zero messages and created nothing (the issue's offline replay: user_count=0, assistant_count=0).

New mcp_server/infrastructure/codex_history.py reads every rollout under $CODEX_HOME/sessions/**/rollout-*.jsonl (CODEX_HOME defaults to ~/.codex) into the record scanner_parse.build_conversation_record already builds for a Claude transcript:

  • project: session_meta.payload.cwd through shared.project_ids.cwd_to_project_id, the function that names Claude project directories, so one repository worked on from both hosts is one domain;
  • typed prompts: event_msg user_message (legacy history) and event_msg item_completed with a UserMessage item (paginated history), the two shapes Codex's own thread preview reads. response_item messages with role user are not read: they also carry the injected <environment_context> / instruction blocks, which no user typed;
  • assistant turns and tool order: transcript_rollout.rollout_activity (ADR-1081), the reader SessionEnd already uses for the same file;
  • source: "codex" on the record (provenance);
  • threads whose session_meta.thread_source is not user (subagent, guardian review, memory consolidation) are skipped, as the Claude scan skips subagent transcripts; a rollout without that field (older Codex) counts as a user thread.

Sources for locations and shapes, read in openai/codex@e7f32cf: codex-rs/utils/home-dir/src/lib.rs (find_codex_home), codex-rs/rollout/src/lib.rs (SESSIONS_SUBDIR), codex-rs/rollout/src/policy.rs (persisted events), codex-rs/rollout/src/list.rs (event_msg_preview), codex-rs/protocol/src/protocol.rs (ThreadSource, UserMessageEvent), codex-rs/protocol/src/items.rs (TurnItem::UserMessage).

handlers/rebuild_profiles.py concatenates both scans; its tool description names the Codex location. scanner_parse._MAX_USER_TEXT_CHARS becomes public MAX_USER_TEXT_CHARS so both readers cap prompt text identically (ADR-0596). The test suite redirects CODEX_HOME with the other real-data roots (tests_py/conftest.py, and the hermetic_claude_dirs fixture), so a developer's real Codex history never reaches a rebuild under test.

Refs #726 (not Closes: remaining slices below).

Type of change

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

Test plan

  • New tests_py/infrastructure/test_codex_history.py (14 tests): legacy and paginated prompts give exact counts (2 prompts, 2 turns, ["exec", "apply_patch"], duration 120000 ms); injected <environment_context> is not a prompt; the Claude reader returns 0/0 on the same records (the defect, pinned); the committed ADR-1081 excerpt maps to -Users-example-project with 2 turns; subagent / guardian_review / memory_consolidation threads skipped; missing thread_source kept; no session_meta, no cwd, or no message → None; CODEX_HOME set / empty / unset; discovery reads rollout-*.jsonl only, in path order, identically on a second scan.
  • New tests_py/handlers/test_rebuild_profiles_codex.py: an isolated Codex-only history (two rollouts, one cwd, no Claude project) → totalSessions == 2, one domain, outcome created with 2 scanned / 2 resulting sessions; a second force rebuild → stored 2, scanned 2, resulting 2 (no duplication).
  • Ran pytest tests_py/handlers tests_py/infrastructure → 3022 passed, 220 skipped, 3 failed: the three tests_py/infrastructure/test_embedding_engine.py tests that import torch, which this container cannot install (the PyTorch wheel host is blocked by its proxy). Unrelated to this diff. The existing Claude fixtures (test_rebuild_profiles.py, test_rebuild_profiles_accumulation.py, test_scanner.py) pass unchanged.
  • ruff check on the changed files: clean; ruff format --check: the only non-conforming files are 3 under tests_py/scripts, identical on main. python scripts/check_craftsmanship.py: OK (tests_py/conftest.py stays at 300 lines).

Not verified here: a real Codex history (none in this container); the PostgreSQL path (no PostgreSQL here; PG-gated tests skipped); Windows cwd forms from Codex (the mapping is the existing cwd_to_project_id, already tested for Windows paths).

Audit notes

  • Layering: infrastructure/codex_history.py imports infrastructure siblings and shared.project_ids only; the handler stays the composition root.
  • Remaining slices of fix: bootstrap cognitive profiles from supported Codex conversation history #726: (2) SessionEnd for Codex creates the domain when it does not exist yet (hooks/session_lifecycle.py:363 still logs only); (3) a Codex session recorded by SessionEnd and later rescanned is counted once across both paths (today the rescan is a full rebuild, so it does not duplicate, but there is no shared session identity between the two); (4) $CODEX_HOME/archived_sessions is not scanned — whether archived threads belong in a profile is an owner decision.

Coding-standards compliance

  • §2.2 Layer dependency direction preserved.
  • §4.1 / §4.2 craftsmanship gate OK (no new baseline entry).
  • §8 no new numeric constant (the text cap reuses ADR-0596's).
  • §9 No dead code.

Breaking changes

None for Claude-only users. A user with Codex history gets new domains (or more sessions in an existing one) on the next rebuild_profiles.

Reviewer checklist

  • CHANGELOG.md updated under Unreleased / Fixed.
  • No secrets / credentials / PII in the diff.

Generated by Claude Code

@cdeust
cdeust marked this pull request as ready for review November 6, 2026 00:28
@chatgpt-codex-connector

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

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T01:25:29.098528Z 35a7e93 New commits
🔒 Security Review ✅ Completed 2026-11-06T01:24:14.120523Z 35a7e93 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

claude Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

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


Review of PR #881: done

No Blockers. The review has three Notes, none needed before merge:

  • Repository paths with ., _ or a space get different project ids from Claude and from Codex.
  • Each rollout is read fully into memory.
  • PRIVACY.md doesn't list Codex history as a data source.

I could not run pytest or ruff in this job; the review comes from reading the code.

Verdict: approve

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f88d5ccae

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +93 to +95
if isinstance(record, dict):
records.append(record)
return records

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stream rollout records instead of retaining the entire transcript

For large Codex sessions, this retains every decoded record—including tool outputs, reasoning, and injected context—in memory until parsing finishes, although most of those payloads are never used. Because rebuild_profiles now scans Codex history unconditionally, a single sufficiently large rollout can exhaust the server's memory and prevent rebuilding even existing Claude profiles. The Claude scanner bounds metadata reads with head/tail sampling, and rollout_activity already streams its input; extract the metadata and capped prompt text incrementally here rather than accumulating the complete transcript.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vrai sur le fond : _read_records garde tout le rollout en mémoire, y compris pour un thread rejeté dès la première ligne, puis rollout_activity le relit. Ce n'est pas un bloqueur au sens d'ADR-1119 (aucun échec reproduit, P2), donc ce n'est pas poussé dans cette PR : une lecture en un seul passage (session_meta, prompts plafonnés à MAX_USER_TEXT_CHARS, tours et outils) ira dans une PR de raffinement de #726, avec la note de la revue Claude sur PRIVACY.md.


Generated by Claude Code

@claude

claude Bot commented Nov 6, 2026

Copy link
Copy Markdown

Review 1 of 3: PR #881 (Codex history in rebuild_profiles, #726 slice 1)

I ran five reviewers (code quality, performance, test coverage, documentation, security) and checked what they found against the code. No Blockers.

  • Layers: infrastructure → infrastructure/shared; the handler is still the composition root.
  • Constants: none new without a source. MAX_USER_TEXT_CHARS reuses ADR-0596. The craftsmanship gate checks numeric constants only, so _USER_THREAD_SOURCE doesn't trip it.
  • Rename: no leftover _MAX_USER_TEXT_CHARS references.
  • Test isolation: CODEX_HOME is overwritten before any mcp_server import, so no test can read a real ~/.codex.
  • Path safety: a crafted session_meta.cwd cannot leave DOMAINS_DIR (profile_store._domain_path rejects it), and persisted text still goes through gated_files redaction.

I could not run pytest or ruff here (the commands need approval in this job); everything above comes from reading the code.

Notes, no action before merge

  • codex_history.py:17-19 and CHANGELOG: one repository from both hosts is one project only when the cwd has no ., _ or space. Claude Code turns those into - in directory names; cwd_to_project_id does not. SessionEnd already has the same gap. A test with a dotted or underscored cwd would pin it.
  • codex_history.py:85-95: each rollout is loaded fully into memory, even threads rejected on line 1, then read again by rollout_activity. A single streaming pass would bound memory like the Claude head/tail read does.
  • PRIVACY.md:11-18 doesn't list $CODEX_HOME/sessions as a source, but Codex prompt text now reaches exampleMessages in the stored profiles. Worth a bullet.

Verdict: approve

…story (#726, slice 1)

rebuild_profiles walked ~/.claude/projects only, and scanner_parse reads
top-level user/assistant records, so a Codex rollout parsed to zero
messages and a Codex-only history created no domain.

infrastructure/codex_history.py reads every user-thread rollout under
$CODEX_HOME/sessions (default ~/.codex/sessions) into the conversation
record scanner_parse.build_conversation_record builds for a Claude
transcript: project from session_meta.cwd through cwd_to_project_id,
typed prompts from event_msg user_message / item_completed UserMessage,
turns and tool order from transcript_rollout.rollout_activity (ADR-1081),
source "codex". Subagent, guardian and consolidation threads are skipped.
Locations and record shapes: openai/codex@e7f32cf.

The test suite now redirects CODEX_HOME with the other real-data roots.

Refs #726

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYtKhqTifXyihe2MEcp
…unit, run 42137170027)

The unit job's diff-scoped mutation gate (ADR-1122) failed on 9ad593b:
surviving mutants in the changed functions of rebuild_profiles.py,
codex_history.py and scanner_parse.py.

- scanner_parse.py leaves the diff: codex_history imports the existing
  _MAX_USER_TEXT_CHARS instead of renaming it, so extract_message_stats
  (76 survivors, untouched logic) is no longer a changed function.
- codex_history reads rollouts as bytes and lets json.loads decode them:
  a non-UTF-8 line is skipped like any malformed line, and the encoding
  arguments whose mutants no test could tell apart are gone. CODEX_HOME is
  read without a default ("" and None were the same falsy value).
- test_codex_history_edges.py: a malformed or non-UTF-8 line does not end
  the read; an empty file, a non-session_meta first record, a non-object
  meta payload, a non-string cwd, non-string or non-object prompt
  payloads and non-text content blocks are skipped; an unreadable rollout
  is reported on stderr and the scan goes on.
- test_rebuild_profiles_wiring.py: the handler passes force and
  replace_accumulated as False, the discovered memories, brain index and
  both scans to the assembler, and reports duration in milliseconds and
  [] for a build without domains.

Local: GITHUB_BASE_REF=main scripts/ci_mutation_diff.py -> 2 file(s),
0 failed, 0 untested.

Refs #726

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5711763c4c

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +175 to +177
try:
record = parse_rollout_conversation(path)
except OSError as exc:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Isolate parsing failures to the affected rollout

If a rollout contains valid JSON with a malformed timestamp, parsing can raise an exception that this OSError handler does not catch. For example, a string timestamp on session_meta followed by timestamp: 1 on a user event reproducibly raises TypeError in extract_metadata_fields. Since rebuild_profiles now scans Codex history unconditionally, that single bad record aborts the entire rebuild, including unrelated Claude profiles. Validate metadata types or catch and report per-file parsing failures before continuing, as the Claude scanner already does in _scan_project_conversations.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduit (TypeError: '<' not supported between instances of 'int' and 'str'), ainsi que la variante RecursionError d'une ligne trop imbriquée relevée par la revue Claude 2. Corrigé dans 35a7e93 : l'intervalle de temps ne lit plus que les timestamp de type chaîne (_time_span, sans extract_metadata_fields), et discover_codex_conversations signale et saute un rollout qui lève RecursionError comme un rollout illisible. Tests : test_a_timestamp_that_is_not_a_string_is_skipped, test_no_string_timestamp_gives_no_time_span, test_a_too_deeply_nested_line_skips_its_rollout_only.


Generated by Claude Code

@cdeust
cdeust force-pushed the fix/726-codex-profile-bootstrap branch from 5711763 to d7289db Compare November 6, 2026 01:11
@claude

claude Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

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


Review 2 of 3: PR #881, done

No Blockers. The review has three Notes, none needed before merge:

  • One bad rollout stops the whole rebuild, because codex_history.py:177 catches only OSError. The Codex bot's P2.
  • Lines that are not valid UTF-8 are dropped from prompts and metadata, but rollout_activity still counts them as turns and tools.
  • The wiring test's docstring says every argument and field is pinned, but some are not.

I could not run pytest or ruff in this job; the review comes from reading the code.

Verdict: approve

cdeust commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

CI autopilot : correctif poussé pour unit / merge (run 42137170027, head 9ad593b).

Diagnostic : les 11773 tests passaient ; c'est la gate de mutation limitée aux fonctions modifiées (ADR-1122, arrivée sur main après l'ouverture de la PR) qui échouait, avec des mutants survivants dans rebuild_profiles.handler, sept fonctions de codex_history.py et scanner_parse.extract_message_stats. Ce dernier n'entrait dans le périmètre qu'à cause du renommage de _MAX_USER_TEXT_CHARS (76 survivants préexistants) : le renommage est retiré et codex_history importe la constante existante. Les autres sont tués par deux nouveaux fichiers de tests (test_codex_history_edges.py, test_rebuild_profiles_wiring.py), et codex_history lit les rollouts en octets (json.loads décode lui-même ; une ligne non UTF-8 est ignorée comme une ligne malformée), ce qui supprime les arguments d'encodage dont aucun test ne distinguait les mutants.

Commits : d7289db (au-dessus de 7b61cf3) ; la branche est rebasée sur main 499c69d (la PR était de nouveau en conflit sur CHANGELOG.md, conflit mécanique), donc le commit de fusion 9ad593b n'est plus dans l'historique, son contenu oui.

Vérifié localement : GITHUB_BASE_REF=main python scripts/ci_mutation_diff.py → 2 file(s), 0 failed, 0 untested ; pytest tests_py/handlers tests_py/infrastructure → 3070 passés (sans test_embedding_engine.py, torch non installable dans ce conteneur) ; gate craftsmanship OK ; ruff propre sur les fichiers modifiés.
Non vérifié : la CI sur d7289db (en cours), PostgreSQL.


Generated by Claude Code

@claude

claude Bot commented Nov 6, 2026

Copy link
Copy Markdown

Review 2 of 3: PR #881 (Codex history in rebuild_profiles, #726 slice 1)

Scope: this review covers only d7289db (the mutant-killing commit). The branch was rebased, so 9ad593b, the commit Review 1 checked, no longer exists; its content now sits in 7b61cf3. Review 1 found no Blockers, so there was nothing to re-check. I ran five reviewers (code quality, performance, test coverage, documentation, security) and checked what they found against the code.

No Blockers.

  • Reverting the rename leaves no stale references. _MAX_USER_TEXT_CHARS is used only in scanner_parse.py and codex_history.py, and no doc or CHANGELOG entry mentions the constant.
  • Layers are unchanged. No file goes over 300 lines and no function over 40.
  • The new tests test what their names say:
    • json.loads on bytes raises UnicodeDecodeError for invalid UTF-8, and that is a subclass of ValueError, so the existing except catches it.
    • The rollout-a.jsonl directory makes open raise IsADirectoryError, which is an OSError.
    • Every patch target is a name the handler actually imports.
    • CODEX_HOME is still redirected by tests_py/conftest.py, so no test reads a real ~/.codex.

I could not run pytest or ruff in this job; everything above comes from reading the code.

Notes, no action before merge

  • codex_history.py:177 only catches OSError, so one bad rollout stops the whole rebuild, Claude profiles included. Two ways it happens: a rollout that mixes string and integer timestamp values raises TypeError at scanner_parse.py:45 (traced by reading), and a deeply nested line raises RecursionError. The Claude scan catches errors per file (scanner.py:199). This is the Codex bot's open P2; the code is from the first commit.
  • codex_history.py:89: _read_records drops lines that are not valid UTF-8, but rollout_activity reads the same file with errors="replace" (transcript_rollout.py:62). On such a line, the prompt and metadata counts and the turn and tool counts come from different sets of lines.
  • test_rebuild_profiles_wiring.py:1-2 says "each argument and each reported field is pinned exactly", but it isn't:
    • force and replace_accumulated_profiles are only tested absent, so a mutated key name still survives.
    • by_project, domainOutcomes and the save_profiles call are never checked.

Verdict: approve

@claude

claude Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

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


Review 3 of 3: PR #881, done

No Blockers. The fixup fixes the open P2: a non-string timestamp or a too-deeply nested line now skips only its own rollout, so the rest of the rebuild goes on. The review has three Notes, none needed before merge:

  • An empty-string timestamp sets startedAt to "" and duration to None.
  • No test covers the docstring's claim that RecursionError is also raised in rollout_activity.
  • Some mutants in discover_codex_conversations may survive.

The whole PR is accepted. I could not run pytest or ruff in this job; the review comes from reading the code.

Verdict: approve

@claude

claude Bot commented Nov 6, 2026

Copy link
Copy Markdown

Review 3 of 3: PR #881 (Codex history in rebuild_profiles, #726 slice 1)

Scope: only 35a7e93 (fixup), the one commit pushed after d7289db. Reviews 1 and 2 raised no Blockers, so there was nothing to re-check. I ran five reviewers (code quality, performance, test coverage, documentation, security) and checked what they found against the code.

No Blockers. The fixup fixes the open P2 from the Codex bot and Review 2:

  • _time_span (codex_history.py:107) skips any timestamp that is not a string. The mixed string/integer TypeError from scanner_parse.py:45 can no longer happen.
  • discover_codex_conversations now catches RecursionError for each rollout (:187). A line nested too deeply skips only its own file. The error comes from _read_records, before rollout_activity reads the file.
  • Each of the three new tests fails if you revert its fix.
  • Layers are unchanged, and both files are under 300 lines.

I could not run pytest or ruff in this job (the commands need approval); everything above comes from reading the code.

Notes, no action before merge

  • codex_history.py:111: "" is a string, so an empty timestamp becomes startedAt == "" and duration becomes None. The old extract_metadata_fields skipped falsy values (if ts:). Fix: add and t to the filter, plus a test.
  • codex_history.py:177: the docstring says RecursionError is raised "and in rollout_activity". No test exercises that, because _read_records raises first.
  • discover_codex_conversations has mutants that may survive: the {exc} text and sorted(...) are never asserted, and no discover-level test includes a rollout that parses to None.

Accepted: the whole PR (7b61cf3, d7289db, 35a7e93). No file or commit has an open Blocker. The Notes above, the single-pass streaming read (Review 1), and the PRIVACY.md bullet can go in the #726 refinement PR.

Verdict: approve

@github-actions
github-actions Bot merged commit 3e963d1 into main Nov 6, 2026
18 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.

2 participants