Skip to content

test(rust): expand coverage for state, threads, and channel prompts#1969

Merged
senamakel merged 3 commits into
tinyhumansai:mainfrom
senamakel:test/smarter-mock-harness-coverage
May 16, 2026
Merged

test(rust): expand coverage for state, threads, and channel prompts#1969
senamakel merged 3 commits into
tinyhumansai:mainfrom
senamakel:test/smarter-mock-harness-coverage

Conversation

@senamakel
Copy link
Copy Markdown
Member

@senamakel senamakel commented May 16, 2026

Summary

  • Add Rust unit coverage for app_state, threads, config, context, and memory conversation-store edge cases.
  • Add channel prompt-builder and runtime-route coverage for prompt injection, skill path fallback, invalid model cache, unknown providers, and per-sender model switching.
  • Add JSON-RPC integration coverage for app_state_update_local_state snapshot round-trips and the current thread-title fallback path.
  • Normalize config.primary_cloud trimming to match the rest of the provider-selector settings.

Problem

  • The targeted testing modules had thin coverage on persistence, fallback handling, prompt shaping, and per-sender routing behavior.
  • Several important paths were only covered at lower layers or not covered at all, which made regressions in workspace-backed state and mock-backed thread behavior harder to catch.
  • The repo also lacked an explicit regression test documenting that the minimal JSON-RPC title-generation path currently falls back before hitting the mock completions backend.

Solution

  • Add focused Rust tests in the existing *_tests.rs files for the affected domains instead of widening the runtime surface.
  • Add JSON-RPC E2E assertions where state round-trips or mock-backed behavior is user-visible.
  • Fix primary_cloud trimming so provider-selector normalization is coherent across config update paths.
  • Keep the thread-title integration test aligned to current behavior by asserting the fallback path rather than claiming a mock-LLM success path that is not happening yet.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/coverage.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge. N/A: full merged diff-cover could not be re-run locally because unrelated existing test failures blocked a complete cargo llvm-cov pass, but the changed Rust slices and JSON-RPC additions were validated directly.
  • Coverage matrix updated — added/removed/renamed feature rows in docs/TEST-COVERAGE-MATRIX.md reflect this change (or N/A: behaviour-only change) N/A: behaviour-only test coverage change.
  • All affected feature IDs from the matrix are listed in the PR description under ## Related N/A: no matrix row changes; targeted coverage only.
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md) N/A: test-only/core-config coverage work, no release-cut UI surface change.
  • Linked issue closed via Closes #NNN in the ## Related section N/A: no linked GitHub issue was provided for this branch.

Impact

  • Runtime impact is limited to one small config normalization change: primary_cloud now trims surrounding whitespace on update.
  • No new network dependencies, migrations, or platform reach changes.
  • Test coverage is stronger on workspace-backed state, thread turn-snapshot cleanup, prompt composition, and channel route-command behavior.

Related

  • Closes: N/A: no linked GitHub issue provided.
  • Follow-up PR(s)/TODOs:
    • Add a true mock-LLM success-path integration test for threads_generate_title; the current minimal JSON-RPC config still falls back before mock completions are hit.
    • Re-run a full cargo llvm-cov / diff-cover pass once unrelated suite failures are addressed.

AI Authored PR Metadata (required for Codex/Linear PRs)

Keep this section for AI-authored PRs. For human-only PRs, mark each field N/A.

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: test/smarter-mock-harness-coverage
  • Commit SHA: a0026ea9

Validation Run

  • pnpm --filter openhuman-app format:check
  • pnpm typecheck
  • Focused tests:
    • cargo test --lib --no-run
    • cargo test --lib 'openhuman::app_state::ops::tests::'
    • cargo test --lib 'openhuman::threads::ops::tests::'
    • cargo test --lib 'openhuman::context::manager::tests::'
    • cargo test --lib 'openhuman::config::ops::tests::'
    • cargo test --lib 'openhuman::context::channels_prompt::tests::'
    • cargo test --lib 'openhuman::channels::routes::tests::'
    • cargo test --test json_rpc_e2e json_rpc_thread_generate_title_falls_back_when_provider_path_is_unavailable
    • cargo test --test json_rpc_e2e json_rpc_app_state_update_local_state_round_trips_into_snapshot
  • Rust fmt/check (if changed): cargo fmt --manifest-path Cargo.toml --all --check
  • Tauri fmt/check (if changed): cargo check --manifest-path app/src-tauri/Cargo.toml (via push hook pnpm rust:check)

Validation Blocked

  • command: LLVM_COV=$HOME/.rustup/toolchains/1.93.0-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/bin/llvm-cov LLVM_PROFDATA=$HOME/.rustup/toolchains/1.93.0-aarch64-apple-darwin/lib/rustlib/aarch64-apple-darwin/bin/llvm-profdata cargo llvm-cov --lib --json --output-path /tmp/openhuman-lib-cov.json
  • error: unrelated existing failures in openhuman::channels::tests::runtime_dispatch::message_dispatch_processes_messages_in_parallel, openhuman::composio::ops::tests::composio_sync_gmail_via_mock_archives_raw_email_and_updates_outcome, openhuman::local_ai::paths::tests::resolve_tts_voice_path_appends_onnx_for_voice_ids, openhuman::local_ai::service::ollama_admin::*, and openhuman::meet_agent::rpc::tests::push_then_poll_returns_audio_after_brain_turn
  • impact: full repo-wide llvm-cov/diff-cover could not be completed locally; changed slices were validated directly instead.

Behavior Changes

  • Intended behavior change: config.primary_cloud now trims surrounding whitespace when updated.
  • User-visible effect: provider-selection settings are normalized more consistently; otherwise this PR is test-focused.

Parity Contract

  • Legacy behavior preserved: thread-title JSON-RPC integration explicitly locks current fallback behavior instead of assuming a mock-LLM success path.
  • Guard/fallback/dispatch parity checks: added tests for thread title fallback/no-op paths, turn-state purge/delete cleanup, channel command fallback/error responses, and generic channel prompt wording.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): N/A
  • Canonical PR: this PR
  • Resolution (closed/superseded/updated): new canonical PR for this branch

Summary by CodeRabbit

  • Bug Fixes

    • Fixed cloud provider configuration to properly trim whitespace from input values.
  • Tests

    • Expanded test coverage across app state management, configuration handling, message routing, file operations, conversation threading, and end-to-end workflows to improve reliability and catch edge cases.

Review Change Stack

@senamakel senamakel requested a review from a team May 16, 2026 22:36
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

📝 Walkthrough

Walkthrough

This PR adds comprehensive test coverage across multiple functional modules: app-state filesystem and identity operations, config settings normalization with trimming, channel routing and model switching, context configuration and prompt building, memory store operations, thread title generation and lifecycle, and end-to-end integration scenarios. One behavior change normalizes whitespace handling in settings.

Changes

Test Coverage Expansion

Layer / File(s) Summary
Config settings trimming and validation
src/openhuman/config/ops.rs, src/openhuman/config/ops_tests.rs
primary_cloud is normalized by trimming whitespace before storage; tests verify trimmed values persist, empty/whitespace inputs clear the field, and baseline_fps is clamped to upper/lower bounds.
App state filesystem and identity operations
src/openhuman/app_state/ops_tests.rs
Tests cover app-state path creation, URL base resolution, stored-state JSON loading/saving with invalid-JSON quarantine, round-trip persistence of encryption keys and onboarding tasks, and cached current-user identity extraction.
Channel message handling and model routing
src/openhuman/channels/routes_tests.rs
RecordingChannel test double captures outbound messages; tests verify invalid model-cache JSON returns empty results, unknown provider commands send error messages with the provider name, and /model commands clear conversation history and persist route selection.
Context configuration and prompt building
src/openhuman/context/channels_prompt.rs, src/openhuman/context/manager_tests.rs
Tests verify ContextManager honors summarizer-model config override, exposes tool-result and markdown-output settings, tracks session-memory state transitions, and updates stats from handle mutations; prompt-building tests cover UTF-8 truncation, missing-file markers, optional-file omission, and channel-specific capability rendering.
Memory store error handling and JSON parsing
src/openhuman/memory/conversations/store_tests.rs
Tests assert update_thread_labels returns errors for missing threads and read_jsonl correctly skips invalid JSON lines while preserving valid messages in order.
Thread title generation, deletion, and turn-state management
src/openhuman/threads/ops_tests.rs
create_thread_with_title helper; tests verify generate_title preserves custom titles and falls back to first-user-message, thread_delete removes turn-state snapshots, threads_purge clears both valid and corrupted files, and turn_state_clear reports cleared=false when absent.
End-to-end integration scenarios
tests/json_rpc_e2e.rs
Two e2e tests: threads_generate_title verifies fallback behavior when provider is unavailable, and app_state_update_local_state verifies encryption-key trimming and onboarding-task persistence through snapshot round-trip.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

working

Poem

🐰 A rabbit hops through tests so bright,
New checkpoints guard each feature's might,
Settings trimmed, threads titled clean,
The finest test suite ever seen!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically summarizes the primary changes: it highlights the expansion of test coverage across three key areas (state, threads, channel prompts) that align with the majority of file modifications in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label May 16, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/openhuman/config/ops_tests.rs (1)

845-855: ⚡ Quick win

Assert all provider fields in the set-phase.

Right now the set-phase only verifies a subset of provider fields. If trimming regresses for agentic_provider, coding_provider, memory_provider, embeddings_provider, heartbeat_provider, or learning_provider, this test can still pass because the clear-phase asserts None for all fields.

Suggested assertion additions
     assert_eq!(cfg.primary_cloud.as_deref(), Some("provider-a"));
     assert_eq!(
         cfg.reasoning_provider.as_deref(),
         Some("provider-reasoning")
     );
+    assert_eq!(cfg.agentic_provider.as_deref(), Some("provider-agentic"));
+    assert_eq!(cfg.coding_provider.as_deref(), Some("provider-coding"));
+    assert_eq!(cfg.memory_provider.as_deref(), Some("provider-memory"));
+    assert_eq!(cfg.embeddings_provider.as_deref(), Some("provider-embed"));
+    assert_eq!(cfg.heartbeat_provider.as_deref(), Some("provider-heartbeat"));
+    assert_eq!(cfg.learning_provider.as_deref(), Some("provider-learning"));
     assert_eq!(cfg.subconscious_provider.as_deref(), Some("provider-sub"));
🤖 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/config/ops_tests.rs` around lines 845 - 855, The set-phase
assertions currently check only inference_url, primary_cloud,
reasoning_provider, and subconscious_provider on cfg; extend that block to
assert that cfg.agentic_provider, cfg.coding_provider, cfg.memory_provider,
cfg.embeddings_provider, cfg.heartbeat_provider, and cfg.learning_provider
(using as_deref() and Some("<expected>")) are set to the expected provider
strings so regressions trimming those fields will fail the test; locate the
existing assertions around cfg.inference_url / cfg.primary_cloud /
cfg.reasoning_provider / cfg.subconscious_provider and add analogous assert_eq!
lines for each of the listed provider fields.
🤖 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.

Nitpick comments:
In `@src/openhuman/config/ops_tests.rs`:
- Around line 845-855: The set-phase assertions currently check only
inference_url, primary_cloud, reasoning_provider, and subconscious_provider on
cfg; extend that block to assert that cfg.agentic_provider, cfg.coding_provider,
cfg.memory_provider, cfg.embeddings_provider, cfg.heartbeat_provider, and
cfg.learning_provider (using as_deref() and Some("<expected>")) are set to the
expected provider strings so regressions trimming those fields will fail the
test; locate the existing assertions around cfg.inference_url /
cfg.primary_cloud / cfg.reasoning_provider / cfg.subconscious_provider and add
analogous assert_eq! lines for each of the listed provider fields.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b94f822f-91b7-4d2a-a3fb-64a655419729

📥 Commits

Reviewing files that changed from the base of the PR and between 40a384e and a0026ea.

📒 Files selected for processing (9)
  • src/openhuman/app_state/ops_tests.rs
  • src/openhuman/channels/routes_tests.rs
  • src/openhuman/config/ops.rs
  • src/openhuman/config/ops_tests.rs
  • src/openhuman/context/channels_prompt.rs
  • src/openhuman/context/manager_tests.rs
  • src/openhuman/memory/conversations/store_tests.rs
  • src/openhuman/threads/ops_tests.rs
  • tests/json_rpc_e2e.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant