feat: wire file_edits/session_agent_policies/display_name to real surfaces - #3442
Conversation
Problem: polylogue-nua7 found session_agent_policies (402,879 rows), file_edits (76,105), and session_refs (18,949) each have a complete, correct, tested read chain that terminates at repository/archive/sessions.py with nothing above it -- zero references outside polylogue/storage/ except their own tests. session_refs was already wired by #3425/#3431 (correlation_view.py); file_edits and session_agent_policies remained unreachable from any CLI/MCP/API surface. Solution: add Polylogue.get_file_edits()/get_agent_policies() to polylogue/api/archive.py (mirroring the existing get_session_events() reader pattern), and wire them into the MCP get(ref, projection=...) dispatcher as two new projections ("file-edits", "agent-policies") alongside the existing "events" projection. No new MCP tool or tool contract needed -- same six-tool "get" operation, new projection values documented in its docstring. Verification: devtools test tests/unit/mcp/test_server_surfaces.py (9 passed, including 2 new tests exercising get(projection="file-edits") and get(projection="agent-policies") through the real MCP tool_manager entrypoint against a real ArchiveStore-written session, not the storage function directly). Ref polylogue-nua7
Problem: polylogue-nua7 found file_edits (76,105 live rows) and session_agent_policies each have a complete, tested read chain that terminates at the repository layer with nothing above it -- reachable only from the MCP get(projection=...) surface added in the prior commit, not from the CLI. Solution: register two new read views following the existing "events"/ "hooks" pattern -- polylogue/cli/messages.py::run_session_file_edits / run_session_agent_policies call the new Polylogue.get_file_edits() / get_agent_policies() API methods; polylogue/cli/read_views/file_edits.py wires them into the read-view invocation/delivery machinery; registered in read_view_handlers.py, read_view_registry.py, and the profile metadata in archive/viewport/profiles.py. Also required updating polylogue/surfaces/projection_spec.py's EvidenceFamily/ READ_VIEW_PROJECTION_FAMILIES maps (a separate registry from the CLI handler registry) -- missing entries there raised "unknown projection view" at runtime. Regenerated docs/cli-reference.md and docs/plans/topology-target.yaml (new polylogue/cli/read_views/file_edits.py module) via devtools render. Verification: devtools test tests/unit/cli/test_file_edits_and_agent_policies_views.py (2 passed, full CliRunner invocation of `read --view file-edits` / `read --view agent-policies` against a real ArchiveStore-written session -- not the storage function or API method in isolation); devtools test tests/unit/cli/test_click_app.py tests/unit/cli/test_completion_matrix.py -k view (14 passed, no registry drift); devtools verify --quick (exit 0); devtools render all --check (all surfaces sync OK). Ref polylogue-nua7
…s/agent_policies helpers Problem: polylogue-nua7 named four helpers with zero references anywhere in the repo outside their own def + __all__ entry (not even a test): queries/file_edits.py get_file_edit / sync_get_file_edits_for_session, queries/session_refs.py sync_get_session_refs, queries/session_agent_policies.py sync_session_agent_policies_batch. Re-verified with rg against the current tree: still zero references after the async get_file_edits/get_agent_policies methods landed on the real MCP/CLI surfaces in the prior two commits -- these sync/ single-row variants were never the read path anything used. Solution: delete all four, plus their now-dangling sqlite3 imports where nothing else in the module used them (file_edits.py, session_refs.py; session_agent_policies.py keeps sqlite3 for _row_to_agent_policy's Row type hint). Verification: devtools test tests/unit/storage/test_unread_wire_batch_v46.py tests/unit/storage/test_repository_agent_policies.py (13 passed); rg for each deleted name across polylogue/ and tests/ (zero hits); mypy --strict on the three touched files (no issues); ruff format/check clean. Ref polylogue-nua7
Problem: polylogue-cgfy AC3 asks that the Claude Code "slug" wire field (1,500 sampled occurrences, e.g. "greedy-squishing-hamming") reach read surfaces so subagent rows carry names instead of "<uuid-prefix>:agent-<suffix>". The parser already captures it into ParsedSession.display_name (polylogue-2qx.4) and the writer persists it into the sessions.display_name column, but neither the Session nor SessionSummary domain model had a display_name field at all -- the value landed durably and was dropped on every read path. Solution, two independent hydration paths both wired: 1. Async repository path (storage/hydrators.py): added display_name to Session/SessionSummary (archive/session/domain_models.py) and their runtime mixins' display_title property (domain_runtime.py, summary_runtime.py) -- display_title now falls back user_title > title > display_name > id[:8], one tier above the raw id truncation. 2. Sync ArchiveStore summary path that backs `find`/MCP get(ref) default projection (storage/sqlite/archive_tiers/archive.py): added display_name to ArchiveSessionSummary, selected it in read_summary's and list_summaries' SQL, and in _summary_from_row made it a title fallback tier ABOVE the existing structural-label fallback (polylogue-cijx.4 decision 3) when no provider title exists -- display_name is real origin evidence (title_source="origin"), stronger than a derived structural label. Wired through api/archive.py::_archive_summary_to_domain so MCP/CLI/API session summaries carry it. Verification: devtools test tests/unit/mcp/test_server_surfaces.py tests/unit/storage/test_session_display_name_reaches_repository.py (12 passed, including a new MCP get(ref) test proving a title-absent session now surfaces its slug through the real ArchiveStore-backed summary path, and a repository-level test proving both Session and SessionSummary carry display_name/display_title correctly, with a second test proving a real title still wins over the slug); devtools test tests/unit/storage/test_title_source_queryable.py tests/unit/storage/test_archive_tiers_write.py tests/unit/storage/test_archive_tiers_archive.py tests/unit/cli/test_query_exec_laws.py (243 passed, no regressions to existing title/summary logic); mypy --strict on all touched files; devtools verify --quick (exit 0); devtools render all --check (sync OK). Ref polylogue-cgfy
…tract suite Problem: devtools test tests/unit/api/test_facade_contracts.py failed test_no_undiscovered_async_methods after this branch's earlier commit added Polylogue.get_file_edits()/get_agent_policies() -- the contract suite enumerates every public async method on Polylogue and fails until each is explicitly categorized. Solution: add both to READ_BY_ID_NONE_METHODS (same category as the get_session_events sibling they were modeled on -- returns None for an unknown session id rather than an empty container). Verification: devtools test tests/unit/api/test_facade_contracts.py -k "no_undiscovered or (method_has_typed_signature and (get_file_edits or get_agent_policies))" (3 passed). Ref polylogue-nua7
Rebasing onto origin/master (which gained 5 new commits during this branch's development, including new modules) left docs/plans/topology-target.yaml stale relative to the rebased tree. Regenerated via devtools render topology-projection.
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Ref polylogue-nua7, polylogue-cgfy, polylogue-pbuh
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7a7462e9e
ℹ️ 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".
| if self.display_name: | ||
| return self.display_name |
There was a problem hiding this comment.
For existing sessions whose nonblank title is a synthetic native ID with title_source='unknown' or 'path'—the live-data case this change is intended to fix—display_title returns that value before it can reach the new display_name branch. Consequently, repository-hydrated Session objects and the twin SessionSummary implementation still display the UUID instead of the slug; apply the same title-source-aware distinction already used by _summary_from_row.
AGENTS.md reference: AGENTS.md:L354-L356
Useful? React with 👍 / 👎.
| git_branch=summary.git_branch, | ||
| git_repository_url=summary.git_repository_url, | ||
| provider_project_ref=summary.provider_project_ref, | ||
| display_name=summary.display_name, |
There was a problem hiding this comment.
This wires display_name only through the summary conversion. The public Polylogue.get_session() path uses _archive_session_to_session(archive.read_session(...)), but ArchiveSessionEnvelope neither selects nor carries sessions.display_name, so full session reads still produce Session.display_name=None and fall back to the raw ID when no title exists. Add the column to both archive-envelope read variants and pass it through the full-session converter.
AGENTS.md reference: AGENTS.md:L351-L353
Useful? React with 👍 / 👎.
| """ | ||
| resolved = await self.repository.resolve_id(session_id) | ||
| resolved_id = str(resolved) if resolved is not None else session_id | ||
| session = await self.repository.get(resolved_id) |
There was a problem hiding this comment.
For a large session, each new evidence reader first calls repository.get, which fetches and hydrates every message, attachment, session event, and tag before issuing the small targeted file_edits or session_agent_policies query. Thus even an empty agent-policy projection can consume memory and latency proportional to the entire transcript, making the new CLI/MCP views impractical for the multi-GiB sessions the archive supports; use a lightweight session-record/summary existence check instead. The same issue is repeated in get_agent_policies.
Useful? React with 👍 / 👎.
# Conflicts: # .beads/issues.jsonl # docs/plans/topology-target.yaml
…#3465) ## Summary Closes out the remainder of polylogue-cgfy's disposition work now that PR #3442 landed the flagship consumption gap (structuredPatch/originalFile/oldString -> file_edits, reachable via API + MCP + CLI `read --view file-edits`). Adds the two remaining genuinely-unread, real-value wire keys (`requestId`, `thinkingMetadata.maxThinkingTokens`) and records the full disposition (read / measured-negative / deliberately-dropped-duplicate) for the rest of the bead's "other unread keys of substance" list in the Claude Code OriginSpec's `fidelity_notes`, satisfying AC1. ## Problem polylogue-cgfy's 2026-07-29 corpus enumeration found 34 of the 70 most common Claude Code wire keys with zero references in `polylogue/sources/`. The structuredPatch/originalFile/oldString cluster (105K/92K/86K rows) and most of the "other keys of substance" list (`stop_reason`, `stop_sequence`, `cache_creation`, `ttftMs`, `todos`, `toolUseResult.sandbox`/`filenames`/`numFiles`, `slug`) were captured by prior sessions on this bead and PR #3442, but two items remained genuinely unread with no recorded disposition either way: `requestId` (1,171 sampled occurrences -- the Anthropic API's per-call request id) and `thinkingMetadata.maxThinkingTokens` (34 occurrences -- the extended-thinking token budget). The bead's remaining lower-value items (`userType`, `sourceToolAssistantUUID`, `hookCount`/`hookInfos`, `toolUseID`) also had no recorded disposition, so AC1 ("every key... classified... recorded in the Claude Code OriginSpec fidelity declaration") was incomplete. Verified each of those lower-value items directly against a real ~/.claude/projects corpus before classifying: - `userType` is the literal string `"external"` on every sampled record (measured negative, same class as the bead's own `usage.service_tier` finding). - `sourceToolAssistantUUID` equals that same record's own `parentUuid`, already captured as `ParsedMessage.parent_message_provider_id` -- a duplicate spelling, not new evidence. - `hookCount`/`hookInfos` describe hook firings already tracked durably (with outcome, which `hookInfos` lacks) in `source.db`'s `raw_hook_events`. - `toolUseID` is already consumed for its real signal (the `progress`/`agent_progress` delegation-edge disposition documented in `code_parser.py`'s module docstring). Also discovered en route: `master` HEAD (5798b3d) failed to import at all (`ImportError: cannot import name 'literal_check'`) -- #3458 deleted it as a supposed zero-call-site dead shim, but #3451 had wired real call sites into it earlier the same day. Fixed and merged separately as #3464 so this branch could even run tests; this branch is rebased on top of that fix. ## Solution - `polylogue/sources/parsers/claude/code_parser.py`: `_message_usage_event_payload` gains a `record` parameter (the top-level record `item`, since `requestId`/`thinkingMetadata` live there, not inside `message`); the sole call site now passes `record=item`. Both new keys ride the existing `message_usage` session-event payload as `request_id`/`max_thinking_tokens`. - **Production callers exercising the new fields**: `Session.session_events` -> CLI `read --view events` (`polylogue/cli/read_views/events.py` -> `run_session_events`, `polylogue/cli/messages.py`) and MCP `get(ref, projection="events")` (`polylogue/mcp/server_cutover.py:905`) already render every `session_events` row generically regardless of `event_type` -- no new consumer code needed, verified by reading both call chains before writing this PR (the generic `events` projection was wired by prior work, not this PR). - `polylogue/sources/origin_specs.py`: new `fidelity_notes` entry on the Claude Code `OriginSpec` recording the full disposition table above. ## Verification - `devtools test tests/unit/sources/test_claude_code_unread_wire_fields.py tests/unit/sources/test_parsers_claude_code_artifacts.py tests/unit/sources/test_origin_specs.py` -> 57 passed - `devtools verify --quick` -> exit 0 (format, lint, mypy, render all --check, schema-versioning policy) - Anti-vacuity: each new field has a paired "field absent" test asserting the key is omitted from the payload, not fabricated Ref polylogue-cgfy <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Enhancements** - Claude Code usage events now capture available request identifiers and maximum thinking-token settings. - Missing metadata continues to be omitted, keeping usage records clean and accurate. - **Quality Improvements** - Added coverage to verify that these Claude Code fields are preserved and handled consistently. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 participant
Summary
Three beads about polylogue evidence that is already captured and persisted, but unreachable from any CLI/MCP/API surface. This PR wires the genuine remainders:
file_edits,session_agent_policies, andsessions.display_name(the Claude Code "slug" wire field).Problem
file_edits(76,272 live rows),session_agent_policies(402,879 rows), andsession_refs(19,024 rows) each had a complete, tested read chain that terminated atrepository/archive/sessions.pywith nothing above it.session_refswas already wired by prior PRs fix(insights): prefer typed PR/session evidence over regex correlation #3425/fix(insights): finish typed session-PR evidence + wire root: filter #3431 (read --view correlation);file_editsandsession_agent_policiesremained unreachable from any CLI/MCP/API surface. Four helpers (get_file_edit,sync_get_file_edits_for_session,sync_get_session_refs,sync_session_agent_policies_batch) had zero references anywhere outside their owndef/__all__entry.slugwire field (1,500 sampled occurrences) is captured intoParsedSession.display_nameand persisted intosessions.display_name, but neitherSessionnorSessionSummarycarried adisplay_namefield at all — the value was dropped on every read path. This is the fix for subagent rows rendering as<uuid-prefix>:agent-<suffix>instead of a human name./realm/db/polylogue/index.db.Solution
polylogue/api/archive.py: addedPolylogue.get_file_edits()/get_agent_policies(), mirroring the existingget_session_events()reader pattern (returnsNonefor an unknown session, an empty list for a session with no matching rows).polylogue/mcp/server_cutover.py): wired both into the existingget(ref, projection=...)dispatcher as two new projection values ("file-edits","agent-policies") alongside the existing"events"projection. No new tool or tool contract needed — same six-toolgetoperation.read --view file-edits/read --view agent-policies, following the existingevents/hooksview pattern — new handler modulepolylogue/cli/read_views/file_edits.py, registered inread_view_handlers.py,read_view_registry.py, and the profile metadata inarchive/viewport/profiles.py. Also required adding entries topolylogue/surfaces/projection_spec.py'sEvidenceFamily/READ_VIEW_PROJECTION_FAMILIESmaps — a separate registry from the CLI handler registry that raises "unknown projection view" if a view is missing there.sqlite3imports where nothing else used them.display_namereachability (polylogue-cgfy), two independent hydration paths:storage/hydrators.py): addeddisplay_nametoSession/SessionSummary(archive/session/domain_models.py) and their runtime mixins'display_titleproperty — now falls backuser_title > title > display_name > id[:8].ArchiveStoresummary path that backsfind/MCPget(ref)default projection (storage/sqlite/archive_tiers/archive.py): addeddisplay_nametoArchiveSessionSummary, selected it inread_summary's andlist_summaries' SQL, and made it a title fallback tier above the existing structural-label fallback (polylogue-cijx.4 decision 3) when no provider title exists —display_nameis real origin evidence (title_source="origin"), stronger than a derived structural label. Wired throughapi/archive.py::_archive_summary_to_domain./realm/db/polylogue/index.db, no code change — measurement only):title_source='unknown'(raw-id/structural-label fallback before this PR).display_name; 6,585 of the 14,717title_source='unknown'sessions (44.7%) now surface a real slug-derived title instead of a raw id or structural label — this PR's concrete, measured improvement.session_eventswithevent_type='claude_pr_link': 19,140 rows (unchanged by this PR — pr-link reader wiring was already resolved by the earlier fix(insights): prefer typed PR/session evidence over regex correlation #3425/fix(insights): finish typed session-PR evidence + wire root: filter #3431 pass; see pbuh's own notes).file_edits: 76,272 rows;session_agent_policies: 402,879 rows;session_refs: 19,024 rows (167 distinct sessions) — all now reachable per point 1-3 above.AC disposition
polylogue-nua7: file_edits/session_agent_policies now have real CLI (
read --view file-edits/agent-policies) and MCP (get(projection=...)) consumers; session_refs already had one (prior PRs, verified unchanged). All four zero-reference helpers deleted. Satisfied.polylogue-cgfy: AC1 (per-key classification recorded in OriginSpec) and AC4 (re-runnable committed enumeration) were already addressed by prior passes per the bead's own notes — not re-verified in this pass, out of this PR's declared surface. AC2's
structuredPatch/originalFile/oldStringpersistence was already done (index v46); this PR adds the missing read side (file_edits reachability, point 1-3 above) — the specific "cijx grading rises from observed to checkpointed" wiring is a separate insights-model change (insights/session_commit.py-adjacent, in this PR's avoid list) and remains open, noted here as a genuine remainder. AC3 (slug reaches read surfaces) is satisfied — see point 5 above, proven with a measured live census (point 6) and end-to-end tests. AC5 (bytes/row counts per key acquired) is partially covered by the live counts in point 6 but not a full per-key report.polylogue-pbuh: AC6 (before/after census) satisfied as a live measurement (point 6) — this is the "after" state; no "before" baseline exists since the parser fix landed in an earlier PR and no un-fixed archive is available to compare against. AC1-AC5 were already resolved in prior passes per the bead's own extensive notes; not re-verified here, out of this PR's declared surface (parsers/claude, assembly_claude_code.py, providers/claude_code*.py were avoided per the task's own instructions).
Verification
devtools test tests/unit/mcp/test_server_surfaces.py— 11 passed, including 3 new tests exercisingget(projection="file-edits"),get(projection="agent-policies"), and the defaultget(ref)display_name fallback through the real MCPtool_managerentrypoint against a realArchiveStore-written session.devtools test tests/unit/cli/test_file_edits_and_agent_policies_views.py— 2 passed, fullCliRunnerinvocation ofread --view file-edits/agent-policiesagainst a realArchiveStore-written session.devtools test tests/unit/storage/test_session_display_name_reaches_repository.py— 2 passed, proving bothSessionandSessionSummarycarrydisplay_name/display_titlecorrectly through the real writer → async repository chain, and that a real title still wins over the slug.devtools test tests/unit/storage/test_unread_wire_batch_v46.py tests/unit/storage/test_repository_agent_policies.py— 13 passed (no regression from dead-helper removal).devtools test tests/unit/storage/test_title_source_queryable.py tests/unit/storage/test_archive_tiers_write.py tests/unit/storage/test_archive_tiers_archive.py tests/unit/cli/test_query_exec_laws.py— 243 passed (no regressions to existing title/summary logic).devtools test tests/unit/api/test_facade_contracts.py -k "no_undiscovered or file_edits or agent_polic"— 3 passed.tests/unit/{api,archive,cli,core,insights,sources,storage,surfaces}/...) — 863 passed, 2 pre-existing unrelated failures (test_archive_tiers_api_raw_artifacts_read_source_tier, 4 parametrized cases oftest_filters_props.py::TestFilterDateParsing) confirmed to reproduce identically with this branch's changes reverted — stale hardcoded date assertions / a clock-hygiene issue inarchive/filter/filters.py, untouched by this PR.mypy --stricton every touched file — no issues.devtools verify --quick— exit 0.devtools render all --check— all surfaces sync OK (regenerateddocs/cli-reference.md,docs/plans/topology-target.yamlfor the newpolylogue/cli/read_views/file_edits.pymodule).Not run: full
devtools verify --all/ whole-directorytests/unitsweep (anti-pattern per repo convention — testmon wasn't seeded in this worktree; relying on the targeted + affected-area sweeps above plus CI's post-merge heavytestsuite).Ref polylogue-nua7, polylogue-cgfy, polylogue-pbuh