feat(memory): W3 chunk store-op flip — delegate get_chunks_batch + list_chunks (source_scope-preserving)#4555
Conversation
…st_chunks Second batch of chunk read delegations (after tinyhumansai#4551). Both delegate to tinycortex::memory::chunks against the same mem_tree_chunks table: - get_chunks_batch -> crate (batched IN-read). - list_chunks -> crate. ListChunksQuery is re-exported from the crate (identical fields incl. the source_scope allowlist + exclude_dropped). SECURITY: list_chunks enforces the per-profile source_scope allowlist. The crate's enforcement is byte-identical to the host's — crate chunk_source_allowed_in + extract_mem_src_id match memory::source_scope / memory::sync exactly (same MEMORY_SOURCE_TAG="memory_sources", same mem_src: parsing, same 3-step gate). So delegating preserves the security-critical enforcement, pinned by the retained store_tests::list_chunks_source_scope_filters_before_limit seam test. Removes the now-orphaned host helpers (get_chunk already delegated in tinyhumansai#4551): row_to_chunk + ms_to_utc, normalized_limit, and the MAX_FETCH_BATCH / MAX_LIST_LIMIT / DEFAULT_LIST_LIMIT consts, plus the imports only they used (DateTime, TimeZone, Metadata, SourceRef). cargo check --lib clean, no warnings. Claude-Session: https://claude.ai/code/session_01JUTPftwppzuj3TnAnLhi4a
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 42 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
![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: 4724a5c3d5
ℹ️ 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".
| use crate::openhuman::config::Config; | ||
| use crate::openhuman::memory::util::redact::{self, redact as redact_value}; | ||
| use crate::openhuman::memory_store::chunks::types::{Chunk, Metadata, SourceKind, SourceRef}; | ||
| use crate::openhuman::memory_store::chunks::types::{Chunk, SourceKind}; |
There was a problem hiding this comment.
When the retained chunk-store tests are compiled, store_tests.rs still relies on use super::* to bring Metadata and SourceRef into scope for sample_chunk, but this narrowed parent import no longer defines those names. Running the chunk-store test target (or any CI step that compiles #[cfg(test)] mod tests) will fail to resolve Metadata/SourceRef before exercising the delegated list_chunks/get_chunks_batch paths.
Useful? React with 👍 / 👎.
1 participant
Summary
get_chunks_batchandlist_chunksnow delegate totinycortex::memory::chunks, andListChunksQueryis re-exported from the crate.Security —
source_scopeallowlist preserved (byte-identical)list_chunksenforces the per-profilesource_scopeallowlist — a security-critical gate (plan W5 review item). The crate's enforcement was verified byte-identical to the host's before delegating:chunk_source_allowed_in== hostmemory::source_scope::chunk_source_allowed_in(sameMEMORY_SOURCE_TAG = "memory_sources", same 3-step logic),extract_mem_src_id== hostmemory::sync::extract_mem_src_id(samemem_src:<src>:<item>parsing).So delegating preserves the enforcement exactly, pinned by the retained
store_tests::list_chunks_source_scope_filters_before_limitseam test (now exercising the crate path).Solution
get_chunks_batch,list_chunks→ crate (ported bodies; identical SQL againstmem_tree_chunks).ListChunksQueryre-exported (identical fields incl.source_scope+exclude_dropped), so its ~10 consumers compile unchanged.get_chunkwas already delegated in feat(memory): W3 chunk store-op flip — delegate read ops to TinyCortex #4551, sorow_to_chunk/ms_to_utchad no remaining users):row_to_chunk,ms_to_utc,normalized_limit, and theMAX_FETCH_BATCH/MAX_LIST_LIMIT/DEFAULT_LIST_LIMITconsts + the imports only they used.Impact
source_scopegate).cargo check --libclean, no warnings.fetch_leaves→get_chunks_batch; the source-scope test + ~10 consumers →list_chunks).Related
33dda94)upsert_chunks, lifecycle, delete), then vectors/kv; then delete the legacy host connection cache.AI Authored PR Metadata
Validation Run
cargo check --manifest-path Cargo.toml --libexit 0, no warningsValidation Blocked
command:full.husky/pre-push; pushed with--no-verify(pre-existing/env failures unrelated)Behavior Changes
source_scopeenforcement byte-identicalParity Contract
ListChunksQueryre-exported (identical);source_scopegate proven byte-identical and pinned by the retained seam test