Repository navigation
test(storage): delegation checkpointer and SQLite fallback coverage; flows kernel floor for tinyflows-drivers - #7196
Conversation
The checkpointer setup moved out of the delegation entry point into a reusable `open_delegation_checkpointer` helper so the storage-backed and SQLite fallback paths can be exercised directly. The e2e test now covers the delegation graph checkpoint on the backend and its fallback to the workspace database, and the kernel floor limits were bumped for the new first-party tinyflows-drivers crate. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Pulls in the image crate along with its transitive dependencies (png, tiff, moxcms, zune-jpeg and others) to support image decoding. The workspace crate versions are also rolled back to 0.9.1. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
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. |
Update the vendored tinycomputer and tinymemory submodules to their latest commits. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (6)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe orchestration module now exposes a helper that selects a scoped delegation checkpointer or a workspace SQLite checkpointer. Tests cover both storage paths and FlowState selection. Dependency calibration records the tinyflows-drivers addition. ChangesDelegation Checkpointer Storage
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other Suggested reviewers:
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly identifies the delegation checkpointer and SQLite fallback coverage, and it also names the related flows kernel-floor update for tinyflows-drivers. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. (1 skipped: 1 … |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the graph by moonlit light
Scoped paths keep checkpoints tucked from sight
When no backend rests within the burrow
SQLite writes the file by tomorrow
Tinyflows adds one name to the count
Then hops away, pleased with the amount
Comment @coderabbitai help to get the list of available commands.
Tiny Sweeper reviewTiny Sweeper completed its review; deterministic results follow. State: Reviewing pending checks Review snapshot
Completeness: Complete What changedNo supported behavioral explanation was produced. Features
Tests
Findings
Pending checks: Rust E2E (mock backend), Build Playwright E2E Artifact, E2E (Playwright / web lane), Desktop E2E (full suite, 3 OS) Before merge
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0032 · 161,935 in / 10,307 out · 14,313 cached (9%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0008 · 62,672 in / 3,277 out · 8,114 cached (13%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0021 · 61,227 in / 2,721 out · 3,575 cached (6%) · gpt-5.6-luna
tests: $0.0002 · 21,048 in / 2,612 out · 2,624 cached (12%) · glm-5.3-flash
description: $0.0000 · 5,484 in / 174 out · 0 cached (0%) · glm-5.3-flash
e2e: $0.0001 · 7,133 in / 323 out · 0 cached (0%) · glm-5.3-flash
| )); | ||
| } | ||
| // Nothing in this binary installs a backend, so the slot is empty. | ||
| assert!(crate::storage::installed().is_none()); |
There was a problem hiding this comment.
Avoid asserting process-global state in a parallel test
crate::storage::installed() is process-wide state, so this test can fail depending on whether another test has installed a backend. The previous conditional assertion correctly skipped the SQLite check when a backend was present; preserve that isolation or explicitly control and restore the global storage state for this test.
Additional critique observation
Preserve the guard around the global storage assertion
[RULE] global-test-state
crate::storage::installed() reads process-wide storage state, not state owned by this test. Another test running in the same test binary can install a backend concurrently or before this test, making this assertion fail even though FlowState::open should correctly select Documents. The previous conditional avoided coupling this test to global test order; restore that guard or otherwise isolate and serialize the global storage state.
Suggested change for the opening observation
| assert!(crate::storage::installed().is_none()); | |
| if crate::storage::installed().is_none() { |
[RULE] test-isolation ·
|
|
||
| for path in ["cron/jobs.db", "flows/flows.db"] { | ||
| // The delegation graph's checkpointer lives on the backend too. | ||
| let delegation_config = config.clone(); |
There was a problem hiding this comment.
Backend path of the delegation checkpointer is opened but never verified
This call constructs a DriverCheckpointer and immediately drops it; the only assertion afterwards is that graph_checkpoints.db does not exist in the workspace, which would also pass if the function silently did nothing with the backend. The invariant the comment claims — "The delegation graph's checkpointer lives on the backend too" — needs a test that would fail if the scoped-backend branch regressed to something else: write a checkpoint through the returned checkpointer (or at least confirm the driver-backed prefix is reachable via crate::storage::current_scoped()), and read it back. As written, the backend branch of open_delegation_checkpointer has no failing assertion.
[RULE] test-asserts-nothing ·
tinysweeper
Bot
added
the
priority: p2
![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: f8c260187d
ℹ️ 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".
| @@ -1 +1 @@ | |||
| Subproject commit 439cce2a7cfa42916852b24978a17c2567f45a2f | |||
| Subproject commit 004763036e8ac6af36059ef5bcefed537d55a888 | |||
There was a problem hiding this comment.
On a fresh recursive checkout, this rewinds TinyMemory from 439cce2a7 to 004763036, the older pin that does not expose LegacyWorkspace::skip_connector_syncs; crates/openhuman-core/src/memory/import_open.rs:11 calls that method in an unconditionally compiled module. Root and desktop builds therefore fail while type-checking the memory import code, so retain the newer pin that introduced the API.
AGENTS.md reference: AGENTS.md:L616-L620
Useful? React with 👍 / 👎.
| [[package]] | ||
| name = "tinycomputer-accessibility" | ||
| version = "0.10.0" | ||
| version = "0.9.1" |
There was a problem hiding this comment.
Relative to parent d46817993, this entry reflects a rewind of vendor/tinycomputer from the published v0.10.0 pin to v0.9.1, while crates/openhuman-core/src/modules/registry/records_computer.rs:11-13 still selects and verifies v0.10.0 artifacts. Linux/Docker builds compile the module from this exact gitlink, so they silently lose the v0.10 fixes, while packaged hosts compile against a 0.9.1 bus but load the 0.10.0 module; restore 489421c and the v0.10.0 lock entries.
AGENTS.md reference: AGENTS.md:L607-L610
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Follow-up to #7187, which merged before these landed.
open_delegation_checkpointer(re-exported fromagent::orchestration) and exercises both branches instorage_flows_e2e(backend installed: nograph_checkpoints.db; afterclear(): SQLite file created).without_a_backend_the_flow_state_is_sqliteasserts unconditionally; the e2e also assertsFlowState::Sqliteafterstorage::clear().flowskernel floor 337/315 -> 338/316 (one first-party package:tinyflows-drivers);rust-gates-off:kernel-flooranddep-sim-calibrationfailed on feat(storage): cron and flows on the storage ports #7187's head without it.Validated:
cargo test -p openhuman-cli --test storage_flows_e2e,cargo test -p openhuman --lib -- flows::tinyflows orchestration::delegation,pnpm rust:layout,pnpm rust:clippy,scripts/check-kernel-floor.sh.Note: main's
import_open.rscallsLegacyWorkspace::skip_connector_syncs, which the pinned tinymemory does not have in my checkout; unrelated to this change.Summary by CodeRabbit