feat(memory): own the source readers, Composio normalisers, content formats, and failure taxonomy - #140
Conversation
The GitHub reader shells out to `gh` and `git` through `tokio::process::Command`. The optional tokio dependency enabled only rt/rt-multi-thread/macros/time/sync, so `process` has to be added before the reader can land. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Moves the three network source readers out of the OpenHuman host and into the engine, behind the `sync` feature. Fetching and parsing a source is engine work by the kernel split criterion: a host whose only memory driver was a third-party backend would not need any of this code. Pure relocation. Only the imports change (`crate::openhuman::…` becomes the crate's own paths, `Config` becomes `MemoryConfig`, `config.workspace_dir` becomes `config.workspace`), plus a thin `SourceReader` wrapper per reader so the bodies keep their existing `Result<_, String>` signatures. The wrapper maps through `MemoryError::Other`, which is `#[error(transparent)]`, so error text round-trips byte-for-byte and callers matching on reader messages are unaffected. `reader_for` deliberately still returns `None` for these kinds. Its line is local-vs-network, not implemented-vs-absent: the host owns scheduling, credentials, and egress budgeting, so a reader that hits the network must be constructed by a caller that has already authorized the fetch, never handed out to the timer-driven workspace pipeline. Module docs updated to say so. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds providers::normalize, the home for provider payload normalisers used by hosts that drive Composio through their own provider abstraction rather than the SyncPipeline implementations alongside it. pick_str lands here verbatim from the host rather than re-pointing callers at common::pick_str, because the two are not the same function: common resolves with Value::pointer and coerces Number to string, this one walks with Value::get and rejects any non-string leaf. Unifying them would silently change what a normaliser emits for numeric fields. Both definitions now document the divergence, and the reject-non-strings case is pinned by a test. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Relocates six provider payload normalisers out of the OpenHuman host: clickup/github/notion/linear normalization.rs -> normalize/<provider>.rs slack/post_process.rs -> normalize/slack_post_process.rs gmail/post_process.rs -> normalize/gmail_post_process.rs plus the two #[path]-included test files, byte-identical. These are pure serde_json Value transforms with no credentials, no network and no scheduling, which kernel.md 4 names as driver-side explicitly. Pure relocation. The only deltas are the three a cross-crate move forces: the pick_str import retargets to super::helpers, pub(crate) widens to pub, and the two #[path] attributes follow their renamed test files. The post_process files are otherwise unchanged; both *_tests.rs are identical byte for byte. Post-processors are named <provider>_post_process because slack.rs and github.rs (the SyncPipeline implementations) already hold those names one directory up, and gmail.rs one directory above that. Co-authored-by: Medulla <medulla@tinyhumans.ai>
…t gates Lands the dependency declarations ahead of the code that needs them so the port itself stays a pure relocation. Both crates are already unconditional host dependencies at these versions, so the host graph is unchanged. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Both are on-disk content formats of the embedded engine: a build whose only driver is a third-party backend has no content root, so neither belongs in the host. Pure relocation from openhuman — the only edit is retargeting two log lines in obsidian_registry onto the crate's own `chunks::redact`, which is byte-identical to the host helper they used. Gated behind the new default-off `obsidian` and `wiki-git` features, matching the existing capability-named convention (`git-diff`, `persona`, `sync`). `wiki-git` enables `git2` directly rather than implying `git-diff`, which would also compile in the unrelated `memory::diff` module. Co-authored-by: Medulla <medulla@tinyhumans.ai>
`FailureCode` / `FailureClass` / `PipelineFailure` / `DegradedState` and the `classify_embed_error` classifier are the engine's own failure vocabulary: a build whose only driver was a third-party external backend would not need them, so they belong here rather than in the host. Pure relocation of `src/openhuman/memory/tree/health/mod.rs` lines 32-420 (taxonomy) and 672-1012 (the 25 taxonomy/classifier tests). No logic change. The only deltas a cross-crate move forces: the module doc block gained a scope paragraph, and the test body was dedented 4 spaces to match the crate's `#[path = "x_tests.rs"]` convention (cf. `fsutil_tests.rs`). Two decisions worth recording: - `FailureCode::remediation_key()` is a fixed table of `memory.health.*` i18n keys, which is host product surface by the same argument that keeps `user_error.rs` in the host. It moves anyway, because `remediation_key` is a serialized field of `PipelineFailure` populated by `PipelineFailure::new` — carving the table out would change the type's wire shape, i.e. a semantics change bundled into a move. The emitted strings are byte-identical, so the wire format and the frontend are untouched. - Named `health` for path parity with the host directory, despite `tinycortex_api::health` already meaning driver liveness (`MemoryHealth`). Different crate, no collision; the module doc calls the ambiguity out. What deliberately did NOT move, and stays in the host: - the process-global degradation atomics and their `mark_*`/`clear_*` API — they drive a host socket broadcast and are read by the `pipeline_status` RPC - `health/doctor.rs` — reads `config.scheduler_gate.mode` - `health/user_error.rs` — its `kind` string is a pinned frontend contract Unconditional, no feature gate: serde/anyhow are already non-optional deps. The `tinycortex-api` dependency floor is unchanged. Co-authored-by: Medulla <medulla@tinyhumans.ai>
The three files ported in from the host in this branch were byte-faithful relocations, so they carried the host's line breaks. Under this crate's rustfmt the widened `pub` signatures now fit on one line and a stray blank line in health.rs is surplus. Formatting only; no code changes. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Renaming it or converting it to a FromStr impl would change the signature as part of a move, which the port was structured to avoid. Co-authored-by: Medulla <medulla@tinyhumans.ai>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 26 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 (48)
📝 WalkthroughWalkthroughThis change adds typed memory health classification, GitHub/RSS/web-page readers, Obsidian and wiki Git storage, and Composio response normalizers with provider-specific tests. ChangesMemory pipeline health
Network-backed source readers
Obsidian and wiki Git content storage
Composio provider normalization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GithubReader
participant GitHubCLI
participant GitHubREST
participant BareRepository
GithubReader->>GitHubCLI: Request repository activity
GitHubCLI-->>GithubReader: Return CLI results
GithubReader->>GitHubREST: Fetch paginated fallback data
GitHubREST-->>GithubReader: Return commits, issues, and pull requests
GithubReader->>BareRepository: Clone or fetch commit history
BareRepository-->>GithubReader: Return commit metadata and content
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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: 8a047da5ea
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (9)
src/memory/store/content/obsidian.rs-47-95 (1)
47-95: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPublish each default atomically.
Line 57 creates the final file before Line 70 writes its contents. If the process or host stops during this interval,
.obsidian/graph.jsonortypes.jsoncan remain empty or partial. Later calls treat that file as user-owned and never repair it.Use
super::atomic::write_if_newso the completed file is published without replacement.Proposed fix
fn write_default_if_missing(obsidian_dir: &Path, name: &str, body: &str) { - use std::io::{ErrorKind, Write}; let target = obsidian_dir.join(name); - let mut file = match std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&target) - { - Ok(f) => f, - Err(err) if err.kind() == ErrorKind::AlreadyExists => return, - Err(err) => { - log::warn!(...); - return; - } - }; - match file.write_all(body.as_bytes()) { - Ok(()) => log::info!(...), + match super::atomic::write_if_new(&target, body.as_bytes()) { + Ok(true) => log::info!(...), + Ok(false) => {} Err(err) => { - ... + log::warn!( + "[content_store::obsidian] stage default {} failed at {:?}: {err:#}", + name, + target + ); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/store/content/obsidian.rs` around lines 47 - 95, Replace the manual OpenOptions/create_new and write_all logic in write_default_if_missing with super::atomic::write_if_new, passing the target path and body so defaults are fully written before publication and never replace an existing file. Preserve the existing idempotent AlreadyExists behavior and logging for creation or write failures as supported by the atomic helper.src/memory/store/content/obsidian_registry.rs-154-178 (1)
154-178: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject paths that contain
..before the ancestor comparison.Line 154 removes trailing separators only.
Path::components()retains..components. Forcontent_root = /vault/../outsideand a registered vault at/vault, this function reportsregistered = truealthough the content root is outside that vault.Reject
ParentDircomponents before comparison. This preserves the documented conservative behavior and prevents an invalid Obsidian deep link.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/store/content/obsidian_registry.rs` around lines 154 - 178, Update is_ancestor_or_equal to return false when either path contains a Component::ParentDir component before performing the prefix comparison. Preserve the existing empty-path, length, and component-boundary checks so paths with parent traversal are rejected conservatively.src/memory/sync/composio/providers/normalize/github.rs-82-94 (1)
82-94: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the GitHub URL shape before creating an identifier.
github_url_to_slugaccepts arbitrary URLs with enough path segments. A non-GitHub URL can produce an identifier such asowner/repo#42.Validate supported hosts and
issuesorpullpath segments before returning a slug. Add a rejection test for an unrelated URL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/github.rs` around lines 82 - 94, Update github_url_to_slug to validate that the URL uses the GitHub host and that the relevant path segment is exactly “issues” or “pull” before constructing the owner/repo#number slug; otherwise return None. Add a test covering an unrelated host and asserting rejection.src/memory/sync/composio/providers/normalize/notion.rs-87-94 (1)
87-94: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject whitespace-only Notion titles.
A title array containing only whitespace returns
Some(" "). Trim the combined title before checking emptiness and return the trimmed value.Add a whitespace-only title test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/notion.rs` around lines 87 - 94, Update the title normalization logic around the combined text in the Notion provider to trim the joined title before checking emptiness, return the trimmed value, and reject whitespace-only titles. Add a test covering a title array whose combined plain_text contains only whitespace.src/memory/sync/composio/providers/normalize/notion.rs-105-111 (1)
105-111: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument public
now_ms.Add item documentation that states the returned unit and the fallback behavior when the system clock precedes the UNIX epoch. As per coding guidelines, “Document public APIs, module contracts, and non-obvious behavior thoroughly.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/notion.rs` around lines 105 - 111, Document the public now_ms function with an item-level doc comment specifying that it returns the current time in milliseconds since the UNIX epoch and returns 0 when the system clock precedes the epoch.Source: Coding guidelines
src/memory/sync/composio/providers/normalize/clickup.rs-37-44 (1)
37-44: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the documented task-ID fallback with the implementation.
extract_task_namedoes not inspectidordata.id. An ID-only task returnsNone.Either add the documented fallback or remove it from the public API documentation. As per coding guidelines, “Document public APIs, module contracts, and non-obvious behavior thoroughly.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/clickup.rs` around lines 37 - 44, Update extract_task_name to match its documented behavior by falling back to the task identifier when no title/name field is present. Inspect both id and data.id after the existing name/title candidates, preserving the Option<String> return contract and leaving the current name precedence unchanged.Source: Coding guidelines
src/memory/sources/readers/rss.rs-1-5 (1)
1-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the module doc:
quick-xmlis not used.The doc states the reader uses
quick-xml. The file parses XML with manual string search only, andquick-xmlis not in the dependency list. Remove the crate name.📝 Proposed fix
//! Fetches and parses an RSS or Atom feed, returning entries as -//! source items. Uses a lightweight XML parser (`quick-xml` via -//! manual parsing) to avoid pulling in heavy feed crates. +//! source items. Parsing is a lightweight, hand-rolled tag scan that +//! avoids pulling in a heavy feed or XML crate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/rss.rs` around lines 1 - 5, Update the module documentation at the top of the RSS/Atom reader to remove the incorrect `quick-xml` reference, while retaining the description that XML is parsed manually with string searches.src/memory/sources/readers/github.rs-60-75 (1)
60-75: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
parse_github_urlaccepts path traversal segments.The check requires exactly two non-empty segments. It does not reject
.or... Forhttps://github.com/../..,ownerandrepobecome.., andgit_cache_dir(Line 494) then builds a path that escapes<workspace>/git_cache. The clone itself fails, but the cache path escape is avoidable with a cheap guard.Validate the segments against the GitHub name character set.
🛡️ Proposed fix
let parts: Vec<&str> = cleaned.split('/').collect(); if parts.len() != 2 || parts[0].is_empty() || parts[1].is_empty() { return Err(format!( "expected https://github.com/<owner>/<repo>, got: {url}" )); } + let valid = |s: &str| { + s != "." && s != ".." && s.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.')) + }; + if !valid(parts[0]) || !valid(parts[1]) { + return Err(format!("invalid owner/repo in GitHub URL: {url}")); + } Ok((parts[0].to_string(), parts[1].to_string()))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/github.rs` around lines 60 - 75, Update parse_github_url to validate both owner and repository segments against the allowed GitHub name character set before returning them, rejecting path traversal values such as "." and ".." and any other invalid characters. Preserve the existing URL parsing and error behavior for valid owner/repository names.src/memory/sync/composio/providers/normalize/gmail_post_process.rs-380-384 (1)
380-384: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAn explicit
nullinmessageTimestampsuppresses theDateheader fallback.
obj.get("messageTimestamp").cloned()yieldsSome(Value::Null)when the key is present with a JSON null.Option::or_elsedoes not run onSome, sopick_header(&obj, "Date")is skipped anddatestays null. The fallback only works when the key is absent entirely.The failure is silent:
dateanddate_localare both dropped for that message, and downstream sorting loses the timestamp. Filter out null before the fallback.🐛 Proposed fix
let date = obj .get("messageTimestamp") .cloned() + .filter(|v| !v.is_null()) .or_else(|| pick_header(&obj, "Date")) .unwrap_or(Value::Null);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process.rs` around lines 380 - 384, Update the date extraction logic around messageTimestamp to treat an explicit Value::Null as missing before applying the pick_header(&obj, "Date") fallback. Preserve existing non-null messageTimestamp values and the final Value::Null default when neither source provides a timestamp.
🧹 Nitpick comments (24)
src/memory/sync/composio/providers/normalize/helpers.rs (1)
50-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove normalizer tests into sibling test modules.
The implementation files mix test code with production code. Keep each test module in its required
<name>_tests.rssibling.
src/memory/sync/composio/providers/normalize/helpers.rs#L50-L87: move tests tohelpers_tests.rs.src/memory/sync/composio/providers/normalize/clickup.rs#L131-L229: move tests toclickup_tests.rs.src/memory/sync/composio/providers/normalize/github.rs#L128-L248: move tests togithub_tests.rs.src/memory/sync/composio/providers/normalize/linear.rs#L150-L300: move tests tolinear_tests.rs.src/memory/sync/composio/providers/normalize/notion.rs#L113-L252: move tests tonotion_tests.rs.As per path instructions, “Keep tests in per-file
<name>_tests.rssiblings … rather than mixing tests into implementation files.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/helpers.rs` around lines 50 - 87, Move the #[cfg(test)] modules out of the implementation files into sibling test files: helpers.rs lines 50-87 to src/memory/sync/composio/providers/normalize/helpers_tests.rs, clickup.rs lines 131-229 to clickup_tests.rs, github.rs lines 128-248 to github_tests.rs, linear.rs lines 150-300 to linear_tests.rs, and notion.rs lines 113-252 to notion_tests.rs. Preserve each module’s existing tests and imports, exposing the tested implementation symbols as needed.Source: Coding guidelines
src/memory/sources/readers/web_page.rs (1)
156-195: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
extract_by_selectormismatches tag prefixes and nested elements.
openis"<{tag}"with no delimiter, so selectorarticlealso matches<articles>. The close search takes the first</{tag}>, so a nested element of the same name truncates the extraction.Require a delimiter after the tag name.
♻️ Proposed fix
while let Some(start) = html[offset..].find(&open) { let abs_start = offset + start; + // Require a delimiter after the tag name so `article` does not match `<articles>`. + let after = html[abs_start + open.len()..].chars().next(); + if !matches!(after, Some(c) if c.is_whitespace() || c == '>' || c == '/') { + offset = abs_start + open.len(); + continue; + } let content_start = match html[abs_start..].find('>') {The module doc already states that full CSS selector support needs
scraper. Nested same-name elements remain unsupported with this approach.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/web_page.rs` around lines 156 - 195, Update extract_by_selector so opening-tag matching requires a valid delimiter immediately after the tag name, preventing selectors such as article from matching articles. Preserve the existing simple-selector behavior and fallback handling; nested same-name elements remain unsupported with this approach.src/memory/sources/readers/rss.rs (4)
127-165: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd the http(s) scheme guard that
web_page.rsuses.
web_page.rsLines 77-83 reject any URL that is nothttp://orhttps://before the fetch.fetch_urlhere does no scheme check, so the two network readers apply different egress rules for the sameurlfield onMemorySourceEntry. Apply the same guard for consistency.🛡️ Proposed fix
async fn fetch_url(url: &str) -> Result<String, String> { + // SSRF guard: only allow http(s) — reject file://, data://, etc. + if !url.starts_with("http://") && !url.starts_with("https://") { + return Err(format!( + "rss source requires an http(s) URL, got: {}", + url.chars().take(64).collect::<String>() + )); + } let client = reqwest::Client::builder()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/rss.rs` around lines 127 - 165, Update fetch_url to validate that url starts with http:// or https:// before constructing or using the HTTP client, matching the scheme guard in web_page.rs; return an error for all other schemes and preserve the existing fetch behavior for valid HTTP(S) URLs.
176-187: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
parse_feeddrops the publication timestamp.
FeedEntrycarriespublished, butSourceItem.updated_at_msis alwaysNone. Downstream change detection cannot tell new entries from old ones. RSSpubDateis RFC 2822, and Atomupdatedis RFC 3339; parse both.♻️ Proposed fix
.map(|e| SourceItem { id: e.id, title: e.title, - updated_at_ms: None, + updated_at_ms: e.published.as_deref().and_then(parse_feed_ts), })/// Parse an RSS `pubDate` (RFC 2822) or Atom `updated` (RFC 3339) timestamp. fn parse_feed_ts(s: &str) -> Option<i64> { chrono::DateTime::parse_from_rfc2822(s) .or_else(|_| chrono::DateTime::parse_from_rfc3339(s)) .ok() .map(|dt| dt.timestamp_millis()) }
chronois already used bygithub.rs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/rss.rs` around lines 176 - 187, Update parse_feed to convert each FeedEntry.published value into SourceItem.updated_at_ms instead of always using None. Add a parse_feed_ts helper that tries RFC 2822 first and RFC 3339 second, returning the parsed timestamp in milliseconds or None when parsing fails, and reuse it in the mapping.
73-93: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
read_item_innerrefetches the whole feed for every item.Each
read_itemcall runsfetch_urlandparse_feed_full. A sync run over 50 entries downloads and parses the feed 50 times. Consider caching the parsed feed per URL for the duration of a sync run, in the same way thatgithub.rsusesLIST_CACHE.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/rss.rs` around lines 73 - 93, The read_item_inner flow currently fetches and parses the entire RSS feed for every item; add a per-URL parsed-feed cache, modeled on github.rs’s LIST_CACHE, and reuse it for the duration of a sync run. Update the fetch/parse path in read_item_inner so only the first read for a URL calls fetch_url and parse_feed_full, while subsequent reads find the requested item from the cached entries.
114-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated host-redaction logic in the two HTTP readers. Both readers strip the scheme and split on
['/', '?', '#']to redact the URL before logging.rss.rswraps this inurl_host;web_page.rsinlines the same expression in thetracing::debug!call. One copy can drift from the other and leak a path or query string.
src/memory/sources/readers/rss.rs#L114-L125: moveurl_hostintosrc/memory/sources/readers/mod.rsas apub(crate)helper next tointo_engine_error, and gate it on thesyncfeature.src/memory/sources/readers/web_page.rs#L85-L94: replace the inline expression withhost = %super::url_host(&url).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/rss.rs` around lines 114 - 125, Deduplicate URL host redaction by moving rss.rs’s url_host helper into readers/mod.rs as a pub(crate) sync-gated helper beside into_engine_error; remove the local copy from src/memory/sources/readers/rss.rs (114-125). In src/memory/sources/readers/web_page.rs (85-94), replace the inline scheme-stripping and delimiter-splitting expression with super::url_host(&url) in the tracing field.src/memory/sources/readers/github.rs (2)
175-198: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBuild the
reqwest::Clientonce.
api_getbuilds a newClientper request. Each build allocates a fresh connection pool and TLS configuration, so pagination over many pages repeats the TCP and TLS handshake.fetch_all_pagescan issue up toGH_MAX_PAGEScalls.Share one client through a
LazyLock.♻️ Proposed fix
+static GH_CLIENT: std::sync::LazyLock<reqwest::Client> = std::sync::LazyLock::new(|| { + reqwest::Client::builder() + .timeout(Duration::from_secs(20)) + .build() + .expect("build GitHub HTTP client") +}); + async fn api_get(path: &str) -> Result<String, String> { let url = format!("https://api.github.com{path}"); - let client = reqwest::Client::builder() - .timeout(std::time::Duration::from_secs(20)) - .build() - .map_err(|e| format!("failed to build GitHub client: {e}"))?; - let resp = client + let resp = GH_CLIENT .get(&url)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/github.rs` around lines 175 - 198, Update api_get to reuse a single reqwest::Client initialized through a LazyLock instead of building a client for each request. Preserve the existing timeout and request headers, and continue returning client-construction errors appropriately while allowing fetch_all_pages to share the connection pool across calls.
24-33: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
LIST_CACHEis global, but the doc describes per-repo clearing.Line 31 states the cache is "Cleared at the start of each
list_itemscall for the same repo." Line 326 clears the whole map for every repo. If two sources sync concurrently, onelist_itemsdiscards the other repo's cached items, andread_itemfalls back to per-item API calls.The keys are already repo-scoped, so correctness holds. Retain only the current repo's keys, or correct the doc.
♻️ Proposed fix
if let Ok(mut cache) = LIST_CACHE.lock() { - cache.clear(); + let prefix = format!("{owner}/{repo}:"); + cache.retain(|k, _| !k.starts_with(&prefix)); }Also applies to: 323-327
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sources/readers/github.rs` around lines 24 - 33, Update the cache reset logic in list_items to remove only entries belonging to the current owner/repository instead of clearing the entire LIST_CACHE map. Preserve cached items for other repositories so concurrent sources do not trigger unnecessary per-item fetches, and keep the existing repo-scoped key format.Cargo.toml (1)
124-127: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUpdate
dirsto the current major version.
dirs6.0.0 is the current release, and no advisories affect the crate. Update the dependency and lockfile from 5.0.1 to 6.0.0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Cargo.toml` around lines 124 - 127, Update the optional dirs dependency declaration from major version 5 to version 6 in the Cargo manifest, and regenerate the lockfile so dirs resolves to 6.0.0 instead of 5.0.1.src/memory/store/content/wiki_git/mod.rs (2)
25-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the
content_pathcontract on the public field.
commit_summariesfails whencontent_pathdoes not start withwiki/summaries/. This constraint is not visible in the public type. Add a field doc so callers learn the contract without readingsummary_repo_path.📝 Proposed doc addition
pub struct SummaryCommitEntry { pub summary_id: String, + /// Content-root-relative path. Must start with `wiki/summaries/`. pub content_path: String,As per coding guidelines: "Document public APIs, module contracts, and non-obvious behavior thoroughly, preferring module-level docs and item docs."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/store/content/wiki_git/mod.rs` around lines 25 - 33, Document the public content_path field in SummaryCommitEntry with its required contract: values must start with wiki/summaries/ for commit_summaries to succeed. Place the documentation directly on content_path and do not alter the struct’s behavior or other fields.Source: Coding guidelines
97-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd context for the empty-repository case.
open_prepared_repocreates the repository when it is missing. A new repository has no HEAD commit. This line then returns a bare git2 error. Add context so the caller learns that no wiki commit exists yet.♻️ Proposed change
- None => repo.head()?.peel_to_commit()?.id(), + None => repo + .head() + .and_then(|head| head.peel_to_commit()) + .context("wiki git has no commit to point the read pointer at")? + .id(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/store/content/wiki_git/mod.rs` at line 97, Update the None branch in open_prepared_repo to handle repositories with no HEAD commit by adding context to the git2 error indicating that no wiki commit exists yet, while preserving normal HEAD-to-commit ID behavior.src/memory/store/content/wiki_git/tests.rs (3)
196-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the second rejection branch in
summary_repo_path.This test covers only the "wiki git only tracks summary nodes" guard. The "summary content path must live under
wiki/" guard has no test. Validation also runs beforeopen_prepared_repo, so a rejected batch must not create a repository. Assert both.💚 Proposed additional test
#[test] fn commit_summary_rejects_non_summary_paths() { let dir = TempDir::new().unwrap(); let err = commit_summaries( dir.path(), &batch("bad", vec![entry("bad", "wiki/notes/one.md")]), ) .unwrap_err(); assert!(err.to_string().contains("only tracks summary nodes")); } + +#[test] +fn commit_summary_rejects_paths_outside_the_wiki_prefix() { + let dir = TempDir::new().unwrap(); + let err = commit_summaries( + dir.path(), + &batch("bad", vec![entry("bad", "summaries/source/L1/one.md")]), + ) + .unwrap_err(); + assert!(err.to_string().contains("must live under")); + assert!( + !dir.path().join("wiki").exists(), + "rejected batch must not create the wiki repo" + ); +}As per coding guidelines: "Add focused unit tests beside the module under
src/."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/store/content/wiki_git/tests.rs` around lines 196 - 205, Add a focused test beside commit_summary_rejects_non_summary_paths that passes a summary entry whose content path is outside wiki/, asserts the “must live under wiki/” error, and verifies the temporary directory contains no repository artifacts because validation occurs before open_prepared_repo.Source: Coding guidelines
263-302: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe test does not exercise the "move latest" behavior in its name.
This test calls
set_read_pointer_tagonce. It confirms that one timestamped tag and onelatesttag exist. It does not confirm that a second call advanceslatestto a new commit and adds a second timestamped tag rather than replacing the first. That accumulate-and-advance behavior is the core read-pointer contract.The
Ok(None)path ofget_read_pointer_tagfor a missing repository is also untested.💚 Proposed additional test
+#[test] +fn read_pointer_latest_advances_and_history_tags_accumulate() { + let dir = TempDir::new().unwrap(); + assert_eq!(get_read_pointer_tag(dir.path(), "agent:default").unwrap(), None); + + let wiki = dir.path().join("wiki"); + let first = wiki.join("summaries/source/L1/one.md"); + std::fs::create_dir_all(first.parent().unwrap()).unwrap(); + std::fs::write(&first, "one").unwrap(); + commit_summaries( + dir.path(), + &batch("queued_seal", vec![entry("one", "wiki/summaries/source/L1/one.md")]), + ) + .unwrap(); + let first_id = set_read_pointer_tag(dir.path(), "agent:default", None).unwrap(); + + std::fs::write(wiki.join("summaries/source/L1/two.md"), "two").unwrap(); + commit_summaries( + dir.path(), + &batch("queued_seal", vec![entry("two", "wiki/summaries/source/L1/two.md")]), + ) + .unwrap(); + let second_id = set_read_pointer_tag(dir.path(), "agent:default", None).unwrap(); + + assert_ne!(first_id, second_id); + assert_eq!( + get_read_pointer_tag(dir.path(), "agent:default").unwrap().as_deref(), + Some(second_id.as_str()) + ); + + let repo = Repository::open(&wiki).unwrap(); + let tag_prefix = format!("refs/tags/read/{}/", hex::encode("agent:default".as_bytes())); + let timestamped = repo + .references() + .unwrap() + .filter_map(|r| r.ok()?.name().map(str::to_string)) + .filter(|name| name.starts_with(&tag_prefix) && !name.ends_with("/latest")) + .count(); + assert_eq!(timestamped, 2, "each advance must add a timestamped tag"); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/store/content/wiki_git/tests.rs` around lines 263 - 302, Extend the test covering set_read_pointer_tag and get_read_pointer_tag to call set_read_pointer_tag again after creating a new commit, then assert latest advances to the new commit while both timestamped tags remain and no commits are created by pointer movement. Add coverage for the missing-repository case, asserting get_read_pointer_tag returns Ok(None).
235-240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the machine-readable commit trailers.
build_commit_messagealso emitsTree-Id,Tree-Scope,Level-Range,Time-Range-Start, andTime-Range-End. These trailers are machine-readable and downstream tools may parse them. This test does not assert them, so a format change would pass unnoticed.💚 Proposed additional assertions
assert!(msg.contains("Token-Count: 123")); + assert!(msg.contains("Tree-Id: tree-1")); + assert!(msg.contains("Tree-Scope: slack:`#eng`")); + assert!(msg.contains("Level-Range: L2")); + assert!(msg.contains(&format!( + "Time-Range-Start: {}", + ts(1_700_000_000_000).to_rfc3339() + ))); + assert!(msg.contains(&format!( + "Time-Range-End: {}", + ts(1_700_003_600_000).to_rfc3339() + ))); assert!(msg.contains("summary-2 L2 children=7 tokens=123"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/store/content/wiki_git/tests.rs` around lines 235 - 240, Extend the assertions in the build_commit_message test to verify the emitted machine-readable trailers Tree-Id, Tree-Scope, Level-Range, Time-Range-Start, and Time-Range-End, using their expected values for this fixture alongside the existing trailer checks.src/memory/sync/composio/providers/normalize/gmail_post_process_tests.rs (4)
335-354: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the no-op and call-order tests for
apply_response_level_markdown.This test covers the success path. Two documented safety properties have no coverage:
- The count-mismatch no-op at lines 121-130 of
gmail_post_process.rs. The doc at lines 90-92 states that a failed split must leavemessages[]untouched soextract_markdown_bodyfalls through. Nothing pins that today.- The documented call order at line 85:
apply_response_level_markdownruns, thenpost_processconsumes the stashed field intomarkdown. No test chains the two functions, so the contract between them is unverified.💚 Proposed test additions
+#[test] +fn apply_response_level_markdown_is_noop_on_count_mismatch() { + let mut data = json!({ + "messages": [ + {"messageId": "m1", "subject": "Hello"}, + {"messageId": "m2", "subject": "World"}, + {"messageId": "m3", "subject": "Third"}, + ] + }); + let before = data.clone(); + super::apply_response_level_markdown(&mut data, "## Hello\nbody A\n---\n## World\nbody B"); + assert_eq!(data, before, "a failed split must not touch messages[]"); +} + +#[test] +fn stashed_slice_becomes_markdown_after_post_process() { + let mut data = json!({ + "messages": [ + {"messageId": "m1", "subject": "Hello", "messageText": "fallback"}, + {"messageId": "m2", "subject": "World", "messageText": "fallback"}, + ] + }); + super::apply_response_level_markdown(&mut data, "## Hello\nbody A\n---\n## World\nbody B"); + post_process("GMAIL_FETCH_EMAILS", None, &mut data); + assert!(data["messages"][0]["markdown"] + .as_str() + .unwrap() + .contains("body A")); + assert!(data["messages"][1]["markdown"] + .as_str() + .unwrap() + .contains("body B")); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process_tests.rs` around lines 335 - 354, Add tests alongside apply_response_level_markdown covering both documented safety properties: verify a response-level Markdown split whose section count mismatches messages leaves the original messages unchanged, and chain apply_response_level_markdown into post_process to verify the stashed markdownFormatted value is consumed into each message’s markdown field in the documented order.
306-311: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover an RFC 2822 date with a non-zero offset.
Line 309 uses
+0000. With a zero offset thewith_timezone(&chrono::Utc)conversion at line 320 ofgmail_post_process.rsis an identity operation. An implementation that dropped the conversion would still pass this test.Assert the converted instant for a non-zero offset.
💚 Proposed test addition
#[test] fn parse_email_date_accepts_rfc3339_and_rfc2822() { assert!(super::parse_email_date("2026-05-31T10:33:00Z").is_some()); assert!(super::parse_email_date("Sun, 31 May 2026 10:33:00 +0000").is_some()); assert!(super::parse_email_date("not-a-date").is_none()); } + +#[test] +fn parse_email_date_converts_rfc2822_offset_to_utc() { + let parsed = super::parse_email_date("Sun, 31 May 2026 16:03:00 +0530").unwrap(); + let expected = super::parse_email_date("2026-05-31T10:33:00Z").unwrap(); + assert_eq!(parsed, expected); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process_tests.rs` around lines 306 - 311, Update parse_email_date_accepts_rfc3339_and_rfc2822 to use an RFC 2822 input with a non-zero offset and assert the returned datetime equals the corresponding UTC instant, verifying the with_timezone(&chrono::Utc) conversion rather than only checking that parsing succeeds.
186-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the fallthrough assertion and cover the snake_case alias.
Line 188 uses
contains.extract_markdown_bodyreturns the trimmedmessageTextverbatim, so exact equality holds.containswould still pass if the whitespace-onlymarkdownFormattedleaked into the result, which is the exact regression this test guards against.Separately,
extract_markdown_bodyreads amarkdown_formattedalias at line 447 ofgmail_post_process.rs. No test exercises that branch.💚 Proposed test changes
post_process("GMAIL_FETCH_EMAILS", None, &mut v); let md = v["messages"][0]["markdown"].as_str().unwrap(); - assert!(md.contains("real body")); + assert_eq!(md, "real body"); } + +#[test] +fn snake_case_markdown_formatted_alias_is_accepted() { + let mut v = json!({ + "messages": [{ + "messageId": "m1", + "subject": "s", + "messageTimestamp": "2026-04-17", + "labelIds": [], + "markdown_formatted": "# aliased body", + "messageText": "fallback should not be used", + "payload": {} + }] + }); + post_process("GMAIL_FETCH_EMAILS", None, &mut v); + assert_eq!(v["messages"][0]["markdown"], "# aliased body"); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process_tests.rs` around lines 186 - 189, Strengthen the assertion in the fallthrough test around post_process("GMAIL_FETCH_EMAILS", ...) to require exact equality with the expected trimmed “real body” value instead of using contains. Add a test case covering extract_markdown_body’s markdown_formatted snake_case alias, verifying that alias is selected and returned correctly.
223-237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThree of the five separator candidates have no coverage.
split_response_markdown_per_message_with_hinttries five boundary patterns at lines 179-185 ofgmail_post_process.rs:\n---\n,\n\n##,\n\n###,\n\n#, and\n***\n. The tests cover the first two. The###,#, and***branches are untested.The candidate order matters for correctness, because an earlier pattern that yields a matching count wins even when a later one is the true boundary. A test per remaining candidate would pin the priority order.
💚 Proposed additional tests
+#[test] +fn split_response_markdown_falls_back_to_h3_marker() { + let md = "### Alice\n\nbody A\n\n### Bob\n\nbody B"; + let slices = super::split_response_markdown_per_message(md, 2).unwrap(); + assert_eq!(slices.len(), 2); + assert!(slices[1].starts_with("### ")); +} + +#[test] +fn split_response_markdown_falls_back_to_asterisk_rule() { + let md = "## Alice\n\nbody A\n***\n## Bob\n\nbody B"; + let slices = super::split_response_markdown_per_message(md, 2).unwrap(); + assert_eq!(slices.len(), 2); + assert!(slices[0].contains("body A")); + assert!(slices[1].contains("body B")); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process_tests.rs` around lines 223 - 237, Add focused tests for split_response_markdown_per_message covering the remaining separator candidates: ### headings, # headings, and *** rules. Each test should construct multiple message sections, verify the expected slice count and bodies, and ensure the fixtures do not accidentally match an earlier candidate so the candidate priority remains exercised.src/memory/sync/composio/providers/normalize/gmail_post_process.rs (6)
85-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueState the concrete ordering constraint.
The current rationale cites lost "ordering signals". The actual mechanism is field lifetime:
reshape_messagereadsmarkdownFormattedand writesmarkdown, and the slim envelope does not retainmarkdownFormatted. If you call this function afterpost_process, the stashed field is never read.📝 Proposed doc wording
/// **Must be called BEFORE [`post_process`]** because `post_process` -/// reshapes `data` into the slim envelope; once `messages[]` carries -/// our slim shape the upstream message ordering is already locked in -/// but we may have lost original ordering signals if any. +/// reshapes `data` into the slim envelope, and the slim message does +/// not retain `markdownFormatted`. A slice stashed after the reshape +/// is never read by `extract_markdown_body`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process.rs` around lines 85 - 88, Update the documentation around the pre-post_process requirement to state the concrete field-lifetime constraint: reshape_message reads markdownFormatted and writes markdown, while post_process removes markdownFormatted from the slim envelope, so the stashed field must be consumed before post_process. Replace the vague ordering-signals rationale without changing behavior.
118-121: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winClone only the subject hints, not the full messages array.
validate_segments_against_hintsreads one field per hint:subject. Line 120 clones every message in full. This function runs beforereshape_fetch_emails, so each element still carries the verbose upstream shape (payload.parts[], the full header list,messageText). For a 50-message fetch this duplicates the whole response body to read 50 short strings.Build subject-only hints instead. The public signature of
split_response_markdown_per_message_with_hintstays unchanged.♻️ Proposed refactor to avoid cloning message payloads
- // Clone hints out of the messages array so the slice borrows - // don't conflict with the upcoming `messages.iter_mut()` mutation. - let hints: Vec<Value> = messages.clone(); + // Copy only the subject hints out of the messages array so the + // slice borrows don't conflict with the upcoming + // `messages.iter_mut()` mutation, and so we don't duplicate the + // verbose upstream MIME payloads. + let hints: Vec<Value> = messages + .iter() + .map(|m| json!({ "subject": m.get("subject").cloned().unwrap_or(Value::Null) })) + .collect();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process.rs` around lines 118 - 121, Update the hint construction before split_response_markdown_per_message_with_hint to build a Vec<Value> containing only each message’s subject field, instead of cloning the full messages array. Preserve the existing hint argument and public function signature, and ensure the subject-only values remain compatible with validate_segments_against_hints.
405-411: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
date_localandlist_unsubscribein the module envelope example.Both keys are conditional additions to the slim envelope. The envelope example at lines 9-25 omits them. A consumer reading the module docs cannot discover either field or learn that both are optional.
Keep the existing key spellings. As per coding guidelines: "Preserve machine-readable IDs and enum wire strings when porting contracts from OpenHuman."
📝 Proposed doc addition at lines 17-20
//! "date": "…", +//! "date_local": "…", // optional; omitted for UTC hosts +//! "list_unsubscribe": "…", // optional; from the message headers //! "labels": ["INBOX", "UNREAD"], //! "markdown": "…body…", -//! "attachments": [ { "filename": "...", "mimeType": "..." } ] +//! "attachments": [ { "filename": "...", "mimeType": "..." } ] // optional🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process.rs` around lines 405 - 411, Update the module envelope example documentation near the existing envelope fields to include the optional date_local and list_unsubscribe keys, preserving their exact machine-readable spellings and showing that each may be absent. Do not change the conditional insertion logic in the post-processing code.Source: Coding guidelines
315-361: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving the date helpers to a sibling module.
The file is 489 lines. The guideline sets the ceiling at 500, so the next added function crosses it.
parse_email_date,EMAIL_LOCAL_TIME_FMT,format_at_tz, andformat_email_local_timeform a self-contained unit with no dependency on the reshape logic. Moving them to a sibling such asgmail_dates.rsreclaims roughly 45 lines and keeps both modules cohesive.This is optional for this PR, since the change is a straight port with unchanged behavior.
As per coding guidelines: "Avoid letting any source file grow beyond 500 lines; split behavior into focused modules before that point."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process.rs` around lines 315 - 361, Move the self-contained date helpers parse_email_date, EMAIL_LOCAL_TIME_FMT, format_at_tz, and format_email_local_time from the current normalization module into a sibling gmail_dates module. Update imports and call sites to preserve their public accessibility and unchanged date parsing/formatting behavior while keeping the normalization module below the 500-line limit.Source: Coding guidelines
276-285: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated container-selection logic.
Lines 279-285 repeat the container selection from
apply_response_level_markdownat lines 98-110, including theget_mut("data").unwrap()re-lookup that works around the borrow checker. Both sites resolve the same contract: find the object that ownsmessages, whether flat or wrapped underdata.Extract one helper and call it from both. The debug log stays at the
apply_response_level_markdowncall site.♻️ Proposed helper extraction
+/// Resolve the object that owns `messages[]`. The Composio response +/// carries it either at the top level or wrapped under `data`. +fn messages_container(data: &mut Value) -> Option<&mut Value> { + if data.get("messages").is_some() { + return Some(data); + } + match data.get_mut("data") { + Some(inner) if inner.is_object() => Some(inner), + _ => None, + } +} + fn reshape_fetch_emails(data: &mut Value) { - // Unwrap an optional `data:` envelope so downstream logic only has - // to deal with one shape. - let container = match data.get_mut("messages") { - Some(_) => data, - None => match data.get_mut("data").and_then(|v| v.as_object_mut()) { - Some(_) => data.get_mut("data").unwrap(), - None => return, - }, - }; + // Unwrap an optional `data:` envelope so downstream logic only has + // to deal with one shape. + let Some(container) = messages_container(data) else { + return; + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process.rs` around lines 276 - 285, Extract the shared container-selection logic from apply_response_level_markdown and reshape_fetch_emails into one helper that resolves the object owning messages, whether directly on the root or under data, while preserving the existing mutable-borrow behavior without duplicated unwrap lookups. Replace both local selection blocks with the helper, and keep the debug log only at the apply_response_level_markdown call site.
142-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the hint documentation onto the function that takes the hint.
split_response_markdown_per_messageat line 160 always passesNoneformessages_hint, yet its doc block at lines 155-159 describes hint validation.split_response_markdown_per_message_with_hintat line 164 is public and carries no doc comment.Split the doc block: keep the boundary-pattern description on both, and move the
messages_hintparagraph to the_with_hintfunction.As per coding guidelines: "Document public APIs, module contracts, and non-obvious behavior thoroughly, preferring module-level docs and item docs."
📝 Proposed doc reorganisation
-/// `messages_hint` is the slim message array from the same response -/// — when present we use the per-message `subject` field to verify -/// each segment really does belong to the message at the same index. -/// Mismatches force a fallback so we never write a wrong-message body -/// to the raw archive. +/// This wrapper performs no subject validation. Use +/// [`split_response_markdown_per_message_with_hint`] when the caller +/// has the message array available. pub fn split_response_markdown_per_message(md: &str, expected_count: usize) -> Option<Vec<String>> { split_response_markdown_per_message_with_hint(md, expected_count, None) } +/// Split a top-level `markdownFormatted` string into per-message +/// segments, validated against the message array. +/// +/// `messages_hint` is the message array from the same response. When +/// present, the per-message `subject` field verifies that each segment +/// belongs to the message at the same index. A mismatch forces a +/// fallback, so a wrong-message body never reaches the raw archive. +/// +/// Returns `Some(slices)` only when a candidate boundary yields exactly +/// `expected_count` segments and validation passes. pub fn split_response_markdown_per_message_with_hint(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/memory/sync/composio/providers/normalize/gmail_post_process.rs` around lines 142 - 168, Reorganize the documentation for split_response_markdown_per_message and split_response_markdown_per_message_with_hint: keep the split-pattern, preamble, and fallback behavior documented on both public functions, but move the messages_hint subject-validation paragraph exclusively onto split_response_markdown_per_message_with_hint. Ensure the no-hint wrapper’s docs do not describe validation it cannot perform.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6753c41-c64e-402b-91cd-2661b6edaefe
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
Cargo.tomlsrc/memory/health.rssrc/memory/health_tests.rssrc/memory/mod.rssrc/memory/sources/mod.rssrc/memory/sources/readers/github.rssrc/memory/sources/readers/mod.rssrc/memory/sources/readers/rss.rssrc/memory/sources/readers/web_page.rssrc/memory/store/content/mod.rssrc/memory/store/content/obsidian.rssrc/memory/store/content/obsidian_defaults/graph.jsonsrc/memory/store/content/obsidian_defaults/types.jsonsrc/memory/store/content/obsidian_registry.rssrc/memory/store/content/wiki_git/mod.rssrc/memory/store/content/wiki_git/tests.rssrc/memory/sync/composio/providers/common.rssrc/memory/sync/composio/providers/mod.rssrc/memory/sync/composio/providers/normalize/clickup.rssrc/memory/sync/composio/providers/normalize/github.rssrc/memory/sync/composio/providers/normalize/gmail_post_process.rssrc/memory/sync/composio/providers/normalize/gmail_post_process_tests.rssrc/memory/sync/composio/providers/normalize/helpers.rssrc/memory/sync/composio/providers/normalize/linear.rssrc/memory/sync/composio/providers/normalize/mod.rssrc/memory/sync/composio/providers/normalize/notion.rssrc/memory/sync/composio/providers/normalize/slack_post_process.rssrc/memory/sync/composio/providers/normalize/slack_post_process_tests.rs
The queue worker now downcasts the anyhow chain to health::PipelineFailure and
maps it onto the queue's JobFailure { code, class }, so unrecoverable codes
(budget_exhausted, auth_invalid, rate_limited, server_error) fail fast instead
of persisting a null failure class and consuming retries.
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Split github.rs into github/{types,git,api}.rs so the orchestrator stays under
the 500-line ceiling. list_items_inner now reads MemorySourceEntry branch/paths:
git log narrows to the configured ref instead of --all and applies pathspecs,
and the REST fallback sends sha and path query params. gh_available is async
with the probe cached in a tokio OnceCell, and tests moved to the sibling
github_tests.rs.
Co-authored-by: Medulla <medulla@tinyhumans.ai>
read_item now runs extract_by_selector when a CSS selector is configured instead of always falling back to the whole page. The fetch installs a custom reqwest redirect policy that re-applies the host/scheme check on every hop and rejects loopback/private/link-local/unique-local IPs plus local hostnames. strip_html_tags gains a strip_script_and_style pre-pass so JS/CSS bodies never reach memory chunks. Tests moved to sibling web_page_tests.rs. Co-authored-by: Medulla <medulla@tinyhumans.ai>
extract_tag now unwraps a surrounding <![CDATA[ ... ]]> wrapper via a new unwrap_cdata helper, and extract_cdata reuses extract_tag. decode_xml_entities decodes & last so escaped entity text like &lt; survives as the literal string instead of being decoded twice into markup. Tests moved to sibling rss_tests.rs. Co-authored-by: Medulla <medulla@tinyhumans.ai>
…andidates remove_nested now prunes a consumed top-level data envelope (and nested message objects) instead of leaving an empty object, so post-processed slack values carry no duplicate verbose tree. extract_issues probes the doubly-nested data/data/issues/nodes shape and top-level issues/nodes alongside the existing candidates. helper docs now describe the pick_str divergence without a broken intra-doc link. Co-authored-by: Medulla <medulla@tinyhumans.ai>
stage_chunks, stage_summary_with_layout, and write_raw_items all call ensure_obsidian_defaults_if_enabled so a fresh content root gets the bundled .obsidian/ graph colour mapping. The wrapper is feature-gated and best-effort (never aborts persistence over a cosmetic default). Tests relocated to sibling obsidian_tests.rs and obsidian_registry_tests.rs, with a feature-gated regression test for the staged graph.json/types.json. Co-authored-by: Medulla <medulla@tinyhumans.ai>
…and symlink loops The wiki-git mutex guard is recovered from a poisoned lock instead of panicking on later operations. set_read_pointer_tag validates a supplied commit resolves in the wiki repo before writing a tag. commit_index_if_changed treats only UnbornBranch as no-parent, surfacing real HEAD errors. stage_summary_dir walks via DirEntry::file_type so symlink loops are skipped instead of recursing to stack overflow. Tests relocated to sibling wiki_git_tests.rs with regression coverage for each case. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3da60caa68
ℹ️ 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".
The SDK job runs RUSTDOCFLAGS=-D warnings cargo doc --all-features --no-deps, which turns rustdoc::private_intra_doc_links into a hard error. Five doc comments referenced private items with intra-doc link syntax: with_detail -> truncate_detail, the github module layout links to the private types/git/api submodules, and web_page linked WebPageReader::read_item_inner. Use plain code spans for those. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa4f25ceeb
ℹ️ 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".
A bare `git clone` records no `remote.origin.fetch` mapping, so a bare `git fetch` without an explicit refspec only touches FETCH_HEAD and leaves `refs/heads/*` at the initial clone — every later sync silently misses new GitHub activity. Pass `+refs/heads/*:refs/heads/*` (with `--prune`) and split the fetch path into `fetch_existing_bare` so it is unit-testable against a local bare repo. Co-authored-by: Medulla <medulla@tinyhumans.ai>
…Comment to types.rs Move list_issues/list_prs/read_issue/read_pr/fetch_issue_comments into a focused issues.rs, keep pagination + merge_commit_batches in api.rs, and relocate IssueComment to types.rs so the shared reader types live together. Call sites in github.rs route through the new issues module; merge_commit_batches gains unit tests for dedupe/ordering. Co-authored-by: Medulla <medulla@tinyhumans.ai>
A hostname whose text is public can still resolve to a loopback, private, link-local, or cloud-metadata address (169.254.169.254) at lookup time. Install a reqwest DNS resolver that only yields globally routable addresses and re-apply the host/scheme check on every redirect hop, so the fetch is pinned to a vetted address. Extract the guard into web_page_ssrf.rs (with sibling tests) to keep web_page.rs under the 500-line guideline. Enables the 'net' tokio feature for the resolver's lookup_host call. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Extract the #[cfg(test)] modules from the five Composio normalizer files (clickup, github, helpers, linear, notion) into per-file <name>_tests.rs siblings, matching the repo convention of keeping tests out of implementation files. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f43ba1ecaf
ℹ️ 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".
…and redundant fetches Replace the web-page-only SSRF module with a shared `ssrf` guard used by both the web-page and RSS readers: scheme/host policy, a DNS resolver that pins connections to globally routable addresses, and per-hop redirect re-checks. `read_body_capped` streams response bodies so the size caps (10 MiB page, 5 MiB feed) are enforced while downloading rather than after the whole body is buffered into memory. The RSS reader also gains two behavior fixes: the parsed feed is cached briefly so a list-then-read sync pass downloads it once instead of N+1 times, and entry pubDate/updated timestamps are parsed into `updated_at_ms` so workspace sync can skip unchanged entries instead of re-reading every item on every pass. web_page extraction now treats an unclosed opening tag as a non-match instead of slicing past the buffer, and `SelectorSpec` moves to web_page/types.rs to keep the reader under the 500-line guideline. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51ff89697b
ℹ️ 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".
… taxonomy - github/git.rs: walk the bare clone's HEAD (default branch) instead of --all when no branch is configured, matching the REST fallback's default-branch scope; docs + tests updated. - github/api.rs: percent-encode branch/path values in the commits list query so & # = spaces inside a filter cannot corrupt the URL; kept / intact for the common path=src/ shape. Fixed clippy unnecessary_sort_by. - rss.rs: url_host now redacts userinfo (user:pass@) via real URL parse, falling back to textual host extraction that still drops credentials. - ssrf.rs/web_page.rs: clippy redundant_closure and needless_borrow fixes. - memory taxonomy + reader/store type definitions moved into dedicated types.rs modules per repo convention: health/, rss/, obsidian_registry/, wiki_git/. Re-exported to keep the public API surface unchanged. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99ec51aa03
ℹ️ 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".
- ssrf.rs: reject IPv4-mapped IPv6 literals (::ffff:127.0.0.1, ::ffff:10.0.0.1) in the hostname check. A literal never goes through DNS resolution, so the PublicOnlyResolver never sees it; the text check now classifies IPv6 literals with the same is_public_ipv6 logic as resolved addresses. Tests cover mapped loopback/private/ link-local blocked and mapped public allowed. - github/api.rs: keep per_page constant at 100 across the pagination walk. GitHub's offset-based pagination is per_page-relative, so a shrinking page size (max not a multiple of 100) re-windowed the offsets and silently skipped rows. Split the walk into collect_pages so the loop is unit-testable; tests pin the constant page size, the no-overlap window, truncation to max, and short-page termination. - linear.rs: add the doubly-nested /data/data/issues/pageInfo cursor path, mirroring the extract_issues envelope shapes so a doubly-nested payload can page. Test added. Co-authored-by: Medulla <medulla@tinyhumans.ai>
…eb_page extract_by_selector lowercased the whole HTML before matching, which corrupted case-sensitive CSS id/class values: a selector like #Main or .ArticleBody could never match the page's id="Main" / class="ArticleBody", so extraction silently fell back to ingesting the entire page. find_next_element now takes the original-cased (script/style-stripped) HTML alongside the lowercased copy: the lowercase copy drives the case-insensitive tag scan, while attr_value reads the attribute value out of the original-cased opening tag (ASCII lowercasing is length-preserving, so byte offsets align). Tag matching stays case-insensitive; id/class value matching is now case-sensitive. Tests: extract_by_selector_preserves_case_for_ids_and_classes (match in correct case, no match + fallback in wrong case) and attr_value_preserves_original_case. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edcce9e148
ℹ️ 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".
Address the four findings from the chatgpt-codex-connector re-review: - ssrf: reject non-public IPv4 literals (multicast, broadcast, reserved, documentation, benchmarking) with the same is_public_ipv4 test the resolved-address guard and IPv6 branch use; a literal never goes through DNS resolution so the text check is the only line of defense. - github/git: refresh the bare clone's HEAD after a fetch so a renamed default branch is repointed (ls-remote --symref + symbolic-ref); the fetch refspec updates refs/heads/* but never HEAD. - rss: a present-but-empty <description>/<content> must not short-circuit the content:encoded/summary fallback; filter empty extractions. - obsidian_registry: absolutize a relative content_root against the CWD before the vault prefix comparison so an already-registered vault is not reported unregistered. Each fix carries a regression test. Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e53bbb6f21
ℹ️ 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 let Some(v4) = ip.to_ipv4_mapped() { | ||
| return is_public_ipv4(v4); | ||
| } | ||
| true |
There was a problem hiding this comment.
When a source URL or redirect targets a network that still routes the deprecated site-local fec0::/10 range, addresses such as http://[fec0::1] pass this classifier: they are neither unique-local/link-local nor multicast/documentation/mapped, so the unconditional true also retains them when returned by DNS. Reqwest can consequently connect to an internal service despite the SSRF guard's globally-routable-only contract; reject site-local and other non-global special-purpose IPv6 ranges rather than relying on this incomplete blacklist.
Useful? React with 👍 / 👎.
| let target = std::path::absolute(content_root) | ||
| .map(|abs| lexically_normalize(&abs)) | ||
| .unwrap_or_else(|_| lexically_normalize(content_root)); |
There was a problem hiding this comment.
When a supported relative content_root contains parent components, such as tmp/../vault, std::path::absolute preserves the .. and lexically_normalize only trims trailing separators. It therefore compares /cwd/tmp/../vault against Obsidian's normalized absolute /cwd/vault and still reports the registered vault as unregistered. Fresh evidence after the prior relative-root fix is that parent components remain in the new target path; collapse . and .. lexically before the component-prefix comparison.
Useful? React with 👍 / 👎.
| if let Some(v) = eq_trimmed.strip_prefix('"') { | ||
| if let Some(end_rel) = v.find('"') { | ||
| return Some(orig_open_tag[value_abs + 1..value_abs + 1 + end_rel].to_string()); | ||
| } | ||
| } else if let Some(v) = eq_trimmed.strip_prefix('\'') { |
There was a problem hiding this comment.
When valid HTML uses unquoted attribute values, for example <div id=main class=content>, this parser returns a value only for double- or single-quoted forms. Consequently selectors such as #main or .content appear not to match and extract_by_selector falls back to ingesting the entire page. Fresh evidence after the previous selector fix is that the current attribute parser has no unquoted-value branch; parse the HTML-permitted unquoted form as well.
Useful? React with 👍 / 👎.
1 participant
Summary
Moves four families of engine code out of the OpenHuman host and into this crate, now that the
memory contract (
tinycortex-api, landed here in #138) lets the host talk to memory through adriver interface rather than reaching into the engine directly.
Under the split criterion the OpenHuman kernel spec uses — "would a build whose only driver is a
third-party backend still need this file?" — all four are engine-side:
memory::sources::readers— thegithub,rss, andweb_pagereaders.memory::sync::composio::providers::normalize— the provider normalisers andpost-processors (pure
serde_json::Valuetransforms).memory::store::content— the Obsidian vault surface and the git wiki mirror, behind twonew optional gates.
memory::health— the pipeline failure taxonomy (FailureCode/FailureClassand retryclassification).
Net effect on the host: −6,429 lines out of its memory tree, with 159 tests relocated here
and passing.
API Or Behavior Changes
No behaviour change. Every move is pure relocation; the two changes that were not pure were
landed as their own commits first, deliberately, so no behaviour change is buried inside a move:
git_cache_dirretargeted to take&Path(049ec85).DEFAULT_BRANCH,ItemKind::prefix,GhPr.comments(2e1339e).New public surface: the moved modules are
pubwhere the host previously had thempub(crate), plusnormalize::helpers::pick_str. Notepick_stris deliberately notproviders::common::pick_str— that one coerces numbers to strings, this one does not. Bothdefinitions carry doc comments explaining the split.
Two new optional features, both default-OFF, so nothing changes for existing consumers:
obsidianstore::content::{obsidian, obsidian_registry}dirswiki-gitstore::content::wiki_githexcargo checkis clean at default,--no-default-features, and--features sync, so both gatesgenuinely compile out.
Tests
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo build --all-targetscargo testThe 159 relocated tests all land and run here:
How relocation purity was checked
git diff -Mis useless here — the moves cross a repository boundary, so git has no shared objectgraph and rename detection finds nothing whether the move is pure or not. Each file was instead
extracted at its pre-deletion revision in the host and byte-diffed against the blob landing here:
obsidian_registry.rsdiffers by exactly 2 lines, retargeting log redaction to
crate::memory::chunks::redact, whichwas itself diffed against the host's helper and is character-for-character the same SHA-256 →
8-hex-char body.
#[path]test attribute. Zero code lineschanged.
pick_strimport, and two#[path]attributes; the 2 test files are byte-identical.trait returns
MemoryEngineResult, so each body moved into a*_innerwith the public methodwrapping via
into_engine_error. Error text is provably preserved:MemoryError::Otheris#[error(transparent)], soto_string()reproduces the originalStringbyte-for-byte. TheSSRF scheme guard survived intact.
Documentation
Module-level docs moved with each family and were extended where the crate needed to state its own
ownership boundary. No separate doc site to update.
Note on this branch's history
It originally carried the
tinycortex-apicommits too. Those were superseded by #138, and main hassince moved ahead of the version this branch had — so PRing it as-is would have reverted part of
#138 (27 insertions against 116 deletions in
api/). The branch has been rebased onto current mainwith those commits dropped; only the engine moves remain.
The consuming side is
tinyhumansai/openhuman#5446, which pins this branch. That PR cannot buildfrom a clean clone until this merges.
Summary by CodeRabbit