refactor(todos): cut over task boards to TinyAgents - #5239
Conversation
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 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 (2)
📝 WalkthroughWalkthroughTask-board persistence and legacy migration now use TinyAgents todo stores through compatibility facades. Scratch tests use asynchronous store clearing, task-board types are re-exported, and converted tool messages explicitly set ChangesTinyAgents task-board integration
Tool-message metadata
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Startup
participant Migration
participant TinyAgentsStore
Startup->>Migration: migrate legacy task boards
Migration->>TinyAgentsStore: import board if absent
TinyAgentsStore-->>Migration: copied or skipped
Migration-->>Startup: migration report
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0577457ee1
ℹ️ 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".
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60e024bd83
ℹ️ 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".
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 732ddcd449
ℹ️ 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".
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eef147fa6
ℹ️ 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".
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fb254ccd5
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/openhuman/agent/task_board.rs (1)
320-325: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHand-rolled hex key duplicates TinyAgents' internal key derivation.
This block hardcodes the store's key scheme, and the same snippet also appears in
src/openhuman/todos/runs.rsaround Line 321. If TinyAgents changes key encoding, the test seeds a row that no longer collides. Prefer a key helper exported fromtinyagents::graph::todos::store(or a single shared test helper) over two copies of the encoding.🤖 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/openhuman/agent/task_board.rs` around lines 320 - 325, Replace the hand-rolled hex encoding used to build the “thread-corrupt” key in the test with the key helper exported by tinyagents::graph::todos::store, or introduce and reuse one shared test helper for this derivation in both task_board.rs and runs.rs. Ensure the seeded key continues to match TinyAgents’ internal key scheme without duplicating encoding logic.src/openhuman/tinyagents/todos.rs (2)
84-93: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winOne failing board aborts the remaining imports.
The
?propagates out of the loop, so boards after the failure are never attempted on this boot (the caller insrc/core/runtime/services.rsjust logs a warning). Prefer per-board error isolation with afailedcounter inTaskBoardMigrationReport.♻️ Suggested change
for board in legacy { - if todos::import_if_absent(&store, board) - .await - .map_err(|error| error.to_string())? - { - report.copied += 1; - } else { - report.skipped += 1; - } + let thread_id = board.thread_id.clone(); + match todos::import_if_absent(&store, board).await { + Ok(true) => report.copied += 1, + Ok(false) => report.skipped += 1, + Err(error) => { + tracing::warn!(thread_id = %thread_id, %error, "[todos] legacy board import failed"); + report.failed += 1; + } + } }🤖 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/openhuman/tinyagents/todos.rs` around lines 84 - 93, Update the legacy board import loop to isolate failures per board instead of propagating errors with ?. Add and maintain a failed counter on TaskBoardMigrationReport, incrementing it when import_if_absent fails, while continuing to process subsequent boards and preserving copied/skipped counts for successful imports.
101-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover
read_legacy_boards' filtering branches.The single test only exercises the happy path. Three untested rules matter: absent
agent_task_boards/returns an empty report,*.runs.jsonledger files are excluded (the ledger stays local — importing one would be a real bug), and unparsable JSON is skipped rather than failing the migration.As per coding guidelines: "Add tests for new or changed behavior; untested code is incomplete."
🤖 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/openhuman/tinyagents/todos.rs` around lines 101 - 147, Expand the migration tests around read_legacy_boards to cover all filtering branches: an absent agent_task_boards directory should produce an empty report, *.runs.json files should be ignored, and malformed JSON files should be skipped without failing migration. Keep the existing happy-path and non-replacement assertions unchanged.Source: Coding guidelines
src/openhuman/todos/ops.rs (2)
75-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRewritten todo flows dropped the repo's verbose, prefixed logging convention. Both new modules replaced previously instrumented code with near-silent paths and no
[todos]-style grep prefix, so branch decisions, mapped errors, and skipped migrations are invisible in logs.
src/openhuman/todos/ops.rs#L75-L102: log eachemit_progressbail-out branch and the error mapped infinish, and prefix the existing drop log with[todos][ops].src/openhuman/tinyagents/todos.rs#L62-L68: raise the skipped-board logs fromdebugtowarnand prefix them with[todos]so unmigrated boards are discoverable next to the migration summary.As per coding guidelines: "New or changed flows must include verbose, grep-friendly diagnostics covering entry/exit, branches, external calls, retries/timeouts, state transitions, and errors".
🤖 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/openhuman/todos/ops.rs` around lines 75 - 102, In src/openhuman/todos/ops.rs lines 75-102, update emit_progress to log each early-return branch, and update finish to log the mapped error; prefix the existing try_send drop message with “[todos][ops]”. In src/openhuman/tinyagents/todos.rs lines 62-68, change skipped-board messages from debug to warn and prefix them with “[todos]”.Source: Coding guidelines
220-230: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the facade's own contract, not just the timestamp format.
Delegation is TinyAgents' business, but three behaviors are owned solely by this file and are now untested:
target()routing (thread → durable store, scratch → in-memory store),snapshot()settingthread_id: NoneforScratchandSome(..)forThread, andemit_progressfiring only for thread boards. A couple of small#[tokio::test]s overadd/listfor bothBoardLocationvariants would cover the routing and snapshot-shape contract.As per coding guidelines: "Add tests for new or changed behavior; untested code is incomplete."
🤖 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/openhuman/todos/ops.rs` around lines 220 - 230, Extend the tests in the tests module to cover the facade contract owned by this file: verify BoardLocation::Thread routes through the durable store while BoardLocation::Scratch uses the in-memory store, snapshot() sets thread_id to Some for Thread and None for Scratch, and emit_progress runs only for thread boards. Add small async tests using add/list for both BoardLocation variants, reusing existing test helpers and public symbols rather than testing only progress_updated_at formatting.Source: Coding guidelines
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/openhuman/agent/task_board.rs`:
- Around line 70-81: Update the doc comment for TaskBoard::put to remove the
outdated description of local normalise_board processing and state that cards
are passed directly to crate_todos::replace, which exclusively performs
normalisation.
- Around line 45-50: Update the updated_at conversion in the Some(mut board)
branch to ensure the returned board always exposes an RFC3339 value: handle both
parse::<i64>() and timestamp_millis_opt(...).single() failures with a canonical
fallback such as Utc::now().to_rfc3339(), and log the original unconvertible
value. Also review the put method’s returned updated_at against the persisted
value so put and subsequent get remain consistent.
In `@src/openhuman/todos/ops.rs`:
- Around line 161-167: Update revise_plan to resolve the unused _feedback
parameter: either pass the actual feedback text into the logging/audit flow and
TinyAgents call so it is consumed, or remove the parameter and update all
callers and the RPC interface to match. Preserve the existing plan revision
result handling through finish.
---
Nitpick comments:
In `@src/openhuman/agent/task_board.rs`:
- Around line 320-325: Replace the hand-rolled hex encoding used to build the
“thread-corrupt” key in the test with the key helper exported by
tinyagents::graph::todos::store, or introduce and reuse one shared test helper
for this derivation in both task_board.rs and runs.rs. Ensure the seeded key
continues to match TinyAgents’ internal key scheme without duplicating encoding
logic.
In `@src/openhuman/tinyagents/todos.rs`:
- Around line 84-93: Update the legacy board import loop to isolate failures per
board instead of propagating errors with ?. Add and maintain a failed counter on
TaskBoardMigrationReport, incrementing it when import_if_absent fails, while
continuing to process subsequent boards and preserving copied/skipped counts for
successful imports.
- Around line 101-147: Expand the migration tests around read_legacy_boards to
cover all filtering branches: an absent agent_task_boards directory should
produce an empty report, *.runs.json files should be ignored, and malformed JSON
files should be skipped without failing migration. Keep the existing happy-path
and non-replacement assertions unchanged.
In `@src/openhuman/todos/ops.rs`:
- Around line 75-102: In src/openhuman/todos/ops.rs lines 75-102, update
emit_progress to log each early-return branch, and update finish to log the
mapped error; prefix the existing try_send drop message with “[todos][ops]”. In
src/openhuman/tinyagents/todos.rs lines 62-68, change skipped-board messages
from debug to warn and prefix them with “[todos]”.
- Around line 220-230: Extend the tests in the tests module to cover the facade
contract owned by this file: verify BoardLocation::Thread routes through the
durable store while BoardLocation::Scratch uses the in-memory store, snapshot()
sets thread_id to Some for Thread and None for Scratch, and emit_progress runs
only for thread boards. Add small async tests using add/list for both
BoardLocation variants, reusing existing test helpers and public symbols rather
than testing only progress_updated_at formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60038e51-57fb-4468-a434-389c5c74e0bb
📒 Files selected for processing (14)
src/core/runtime/services.rssrc/openhuman/agent/message_convert.rssrc/openhuman/agent/task_board.rssrc/openhuman/agent/tools/todo.rssrc/openhuman/tinyagents/mod.rssrc/openhuman/tinyagents/todos.rssrc/openhuman/todos/README.mdsrc/openhuman/todos/crate_adapter.rssrc/openhuman/todos/invariant_tests.rssrc/openhuman/todos/mod.rssrc/openhuman/todos/ops.rssrc/openhuman/todos/runs.rssrc/openhuman/todos/store.rsvendor/tinyagents
💤 Files with no reviewable changes (4)
- src/openhuman/todos/store.rs
- src/openhuman/todos/mod.rs
- src/openhuman/todos/invariant_tests.rs
- src/openhuman/todos/crate_adapter.rs
There was a problem hiding this comment.
senamakel has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Summary
tinyagents::graph::todosauthoritative for task-board types, normalization, persistence, CRUD, plan decisions, session links, and atomic claims.agent_task_boards/*.jsonstorage.Problem
OpenHuman retained a second task-board model and CRUD implementation after TinyAgents gained its own todo module. The two implementations duplicated schema conversion, normalization, locking, storage, and invariants, creating drift risk and making TinyAgents only nominally authoritative.
Solution
OpenHuman now re-exports TinyAgents board types and delegates every board operation to TinyAgents.
openhuman::todos::opsonly maps OpenHuman board locations/snapshot shape and emits app-specific progress events. The RPC controllers, granular tools, and run ledger remain compatible. A TinyAgentsimport_if_absentprimitive preserves the existing fail-closed, non-destructive legacy migration contract.Submission Checklist
Impact
Desktop, CLI, RPC, and agent-tool callers retain their existing method and tool names. Persisted TinyAgents boards remain authoritative; legacy file boards are copied only when no TinyAgents value exists. Scratch boards now use TinyAgents
InMemoryStore. No new network, security, or permission behavior is introduced.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
refactor/todos-tinyagents-cutover60e024bd8Validation Run
cargo check --tests.cargo fmt --check,GGML_NATIVE=OFF cargo check, andGGML_NATIVE=OFF cargo check --tests.Validation Blocked
command:GGML_NATIVE=OFF cargo test --lib todos --no-fail-fasterror:externally terminated with exit 143 during the large OpenHuman test-binary link while other repository builds were running.impact:test code compiled successfully throughcargo check --tests; CI will run the linked suites in an isolated runner.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Documentation