[sanil-23]

Summary

• Hardens a flaky memory::ops unit test that intermittently reds the Rust Core Coverage (cargo-llvm-cov) check.
• memory_ingestion_status_reflects_initialized_client_snapshot asserted an absolute queue_depth == 1, but the queue is a process-global AtomicUsize shared (and never reset) across every memory::ops test — so residue from a prior test's in-flight ingestion makes it flake (CI observed 2, expected 1).
• Fix: assert the delta (baseline_depth + 1) instead.

Problem

memory::ops tests share one process-global memory client (one SQLite DB, one IngestionState) pinned by ensure_shared_memory_client. GLOBAL_MEMORY_TEST_LOCK serializes the test bodies, but IngestionState.queue_depth (an AtomicUsize, read live by snapshot()) is not reset between tests. A prior test that leaves the queue non-zero — or whose async ingestion outlives the body-lock — makes the absolute queue_depth == 1 assertion fail under cargo-llvm-cov's slower parallel timing. This is a real test-isolation bug, independent of any product code; the failing test passes deterministically in isolation.

Solution

Capture baseline_depth = state.snapshot().queue_depth before enqueue(), then assert status.queue_depth == baseline_depth + 1. The other snapshot fields (running, current_document_id, …) are overwritten per call by mark_running, so they stay deterministic and were left unchanged — only the additive queue_depth needed hardening.

Note on verification: this is a low-frequency (~1/9500) flake that does not reproduce locally, so the fix is validated by reasoning + the test passing, not by reproducing the failure. The change is strictly more correct regardless (an absolute assertion against shared, un-reset global state is wrong by construction).

Submission Checklist

• Tests added or updated (happy path + failure/edge) — N/A new test: this is a test-correctness fix to an existing test; verified it passes (cargo test --lib memory_ingestion_status_reflects_initialized_client_snapshot → ok).
• Diff coverage ≥ 80% — changed lines are executed test code (covered when the test runs); comment lines are non-executable. diff-cover gate validated in CI.
• Coverage matrix updated — N/A: test-only fix, no feature change.
• All affected feature IDs from the matrix listed under ## Related — N/A: no matrix rows affected.
• No new external network dependencies introduced — confirmed.
• Manual smoke checklist updated — N/A: test-only fix, no runtime surface changed.
• Linked issue closed via Closes #NNN — N/A: no tracked issue at open (a follow-up issue for the related tool_memory contention facet is referenced below).

Impact

• Platforms: all (test-only). No runtime/behaviour change. Reduces flakiness of the Rust Core Coverage check.

Related

• Closes: N/A — surfaced while investigating CI flakes on PR fix(agent): prevent orphaned tool messages from reaching the provider #2717.
• Follow-up PR(s)/TODOs: memory::ops tests flake under cargo-llvm-cov from shared global client (queue residue + SQLite write contention) #2722 covers the deeper facet — memory::ops::tool_memory test DB-write contention on the same shared client, where the body-lock doesn't gate the SQLite connection / async client lifecycle. That needs a riskier isolation change (quiesce background ingestion under the lock, per-test isolated client, or SQLite busy-retry) and is not addressed here.

---

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

Linear Issue

• Key: N/A (not a Linear/Codex PR)
• URL: N/A

Commit & Branch

• Branch: fix/memory-sync-queue-depth-flake
• Commit SHA: e6670bc9

Validation Run

• pnpm --filter openhuman-app format:check — N/A: no app/ (TS) files changed.
• pnpm typecheck — N/A: no TS changed.
• Focused tests: cargo test --lib memory_ingestion_status_reflects_initialized_client_snapshot → 1 passed.
• Rust fmt/check (if changed): cargo fmt clean; compiles.
• Tauri fmt/check (if changed): N/A: no app/src-tauri changes.

Validation Blocked

• command: full pnpm test:rust / coverage suite + Tauri shell pre-push check
• error: local env lacks vendored tauri-cef CLI + pnpm/node toolchain (documented project constraint)
• impact: focused test compiles + passes locally; full suite validated in CI.

Behavior Changes

• Intended behavior change: none (test-only).
• User-visible effect: none; reduces CI flakiness.

Parity Contract

• Legacy behavior preserved: the test still verifies the ingestion-status snapshot reflects an enqueued+running job; only the depth assertion is made residue-robust.
• Guard/fallback/dispatch parity checks: N/A.

Duplicate / Superseded PR Handling

• Duplicate PR(s): none (note: a stale, unrelated fix/memory-ops-test-isolation branch exists on the fork from earlier work; this PR uses a distinct branch and base).
• Canonical PR: this
• Resolution (closed/superseded/updated): N/A

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Updated test robustness to properly handle persisted state across test executions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 76f81e06-596e-4712-b677-f3679428e657

📥 Commits

Reviewing files that changed from the base of the PR and between 0fddf11 and e6670bc.

📒 Files selected for processing (1)

• src/openhuman/memory/ops/sync.rs

---

📝 Walkthrough

Walkthrough

This PR fixes a test flakiness issue in the memory ingestion sync module. The memory_ingestion_status_reflects_initialized_client_snapshot test previously assumed an absolute queue depth of 1, but process-global ingestion state persists across test runs. The test now captures a baseline queue depth before enqueueing and asserts relative to that baseline.

Changes

Test isolation for memory ingestion state

Layer / File(s)	Summary
Fix ingestion status queue depth assertion for test isolation src/openhuman/memory/ops/sync.rs	Test captures baseline queue_depth before enqueueing and updates final assertion from hardcoded 1 to baseline_depth + 1 to account for process-global state persisting across prior test runs.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• tinyhumansai/openhuman#2649: Adjusts the same sync.rs test file to handle process-global memory state leaking across tests.
• tinyhumansai/openhuman#2451: Changes to ingestion queue depth handling (bounded try_send() and rollback) that directly relate to the queue_depth behavior being tested.
• tinyhumansai/openhuman#2444: Adjusts bounded-queue accounting (increment/decrement on submit/drop) that impacts the queue_depth state this test validates.

Suggested labels

rust-core, memory

Suggested reviewers

• graycyrus
• senamakel

Poem

A test once assumed its state pure and clean,
But global queues whispered of tests in between,
Now baseline depth holds the truth that was hidden,
And flaky assertions obey their new bidding! 🐰✨

🚥 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 describes the main change: hardening a flaky test by making it resilient to process-global queue_depth state persisting across tests.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% 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.}