chore: Merge the insights packages into ordinary layering and delete de… - #4614
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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
Register the renamed analysis package in the root namespace
After this package replacement, code that does import polylogue and then accesses polylogue.analysis receives AttributeError: the root package's lazy-submodule table still contains the now-deleted insights name and has no analysis entry. The stale entry also makes polylogue.insights fail with ModuleNotFoundError, so update the root lazy-submodule registration when adding this replacement package.
ℹ️ 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".
| r.upload_origin, | ||
| r.direction, | ||
| r.producer_ref, | ||
| r.message_id, | ||
| r.session_id |
There was a problem hiding this comment.
When an attachment row has direction='model_output' and a non-null producer_ref, load_sync_batch() now omits both columns and constructs AttachmentRecord with their None defaults, so the exported hydrate_sessions() path returns incorrect attachment provenance. load_async_batch() still preserves these fields through get_attachments_batch(), leaving synchronous and asynchronous rebuilds with different canonical session inputs; retain the two selected columns and constructor assignments here.
AGENTS.md reference: AGENTS.md:L170-L174
Useful? React with 👍 / 👎.
| |------|------|-----------------| | ||
| | **Archive Substrate** | Owns stored meaning: acquisition, parsing, persistence, query | `sources/`, `pipeline/`, `storage/`, `archive/`, `operations/` | | ||
| | **Derived Read Models** | Stored insights computed over the archive | `insights/`, `storage/insights/session/` | | ||
| | **Derived Read Models** | Stored insights computed over the archive | `insights/`, `storage/derived/session/` | |
There was a problem hiding this comment.
The updated primary-module row still directs contributors to insights/, but this commit deletes that directory and replaces it with analysis/; following this architecture map therefore leads to a nonexistent implementation home. The same stale first-home appears in docs/code-navigation.md, so update the product-layer half of the rename as well as the storage half.
AGENTS.md reference: AGENTS.md:L203-L210
Useful? React with 👍 / 👎.
Status counts read product tables without requiring them to exist. The reads were unreachable behind the insight_materialization KeyError; removing that exposed them, so an archive whose derived tables are not built yet raises "no such table" instead of reporting zero. Four count descriptors read a product table they did not gate on. Two relations are query-time views over session_profiles and session_work_events: sqlite_master lists a view whether or not its body's tables exist, so presence alone never made them readable. The view dependency cannot be read off a descriptor's query text, so it is declared once and every gate expands through it. A descriptor now gates on the set of relations its query reads, and an import-time check refuses any descriptor whose query names a product table it does not gate on. Also repoint two test references at storage.derived, renamed from storage.insights in #4614; the stale import failed collection on master. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Summary
Rebase the insights-to-analysis and storage-insights-to-storage-derived package merge onto current master and preserve the Hermes verification prerequisite.
Problem
The package merge had to be replayed across current master changes, including work-effects and session-profile updates. The rebased tree must retain current master behavior while removing stale package references.
Solution
Resolved the rename-aware conflicts, retained the repository-subdirectory and non-repository work-effects coverage, restored Hermes verification coverage, removed the empty storage test package, and corrected remaining analysis and derived-storage references.
Verification
uv run devtools verify --quickpassed. The pre-push quick verification passed all gates, including format, lint, mypy, generated surfaces, layering, patterns, doc commands, schema versioning, oracle integrity, consumer reachability, timestamp doctrine, and schema privacy.Residual risk
The full test corpus and seeded CLI/MCP/API parity harness were not run in this rebase operation.