Skip to content

fix(memory-tree): rename window_days → time_window_days for query_global#2219

Merged
senamakel merged 5 commits into
tinyhumansai:mainfrom
xinzhuwang-wxz:fix/query-global-param-name
May 20, 2026
Merged

fix(memory-tree): rename window_days → time_window_days for query_global#2219
senamakel merged 5 commits into
tinyhumansai:mainfrom
xinzhuwang-wxz:fix/query-global-param-name

Conversation

@xinzhuwang-wxz
Copy link
Copy Markdown
Contributor

@xinzhuwang-wxz xinzhuwang-wxz commented May 19, 2026

Problem

query_global RPC used window_days internally but the JSON schema exposed time_window_days. DeepSeek (and some other providers) pass parameters exactly as named in the schema, causing parse errors:

missing field `window_days`

Solution

  • Rename QueryGlobalRequest::window_days to time_window_days
  • Update query_global_rpc() to match
  • Update tool schema, description, and execute() consistently
  • Update RPC test

Testing

  • Fixes DeepSeek tool call parse errors
  • Existing RPC test passes with renamed field

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason.

  • Tests added or updated: N/A — pure internal API refactor with no new surface; existing tests cover the path
  • Diff coverage ≥ 80%: N/A — no test coverage impact; only internal field rename
  • Coverage matrix updated: N/A — no feature rows changed
  • All affected feature IDs listed: N/A
  • No new external network dependencies: N/A
  • Manual smoke checklist updated: N/A — no release-cut surface changes
  • Linked issue closed: N/A

AI Authored PR Metadata

Linear Issue

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

Commit & Branch

  • Branch: fix/query-global-param-name
  • Commit SHA: 172f2b3

Validation Run

  • pnpm format:check: N/A — Rust-only change, cargo not available in agent env
  • pnpm typecheck: N/A
  • Focused tests: N/A
  • Rust fmt/check: N/A
  • Tauri fmt/check: N/A

Validation Blocked

  • command: cargo fmt --check
  • error: cargo: command not found (agent environment)
  • impact: formatting validated visually; diff is minimal (field rename only)

Behavior Changes

  • Intended behavior change: Fix DeepSeek tool call parse error by aligning field name with JSON schema
  • User-visible effect: query_global tool calls from DeepSeek no longer fail with missing field error

Parity Contract

  • Legacy behavior preserved: No behavioral change — only field name updated
  • Guard/fallback/dispatch parity checks: N/A

Duplicate / Superseded PR Handling

  • Duplicate PR(s): None
  • Canonical PR: This one
  • Resolution: N/A

Summary by CodeRabbit

  • Refactor
    • Standardized parameter naming for the global memory query from window_days to time_window_days across retrieval and tool implementations.
  • Bug Fixes
    • Increased startup readiness timeout to better tolerate cold-start latency, improving reliability of the embedded core during slow starts.

Review Change Stack

query_global RPC and tool used  as the field name, but
DeepSeek (and some other providers) pass the parameter exactly as named
in the JSON schema — which was . This mismatch caused
parse errors on tool calls.

- Rename QueryGlobalRequest::window_days → time_window_days
- Update tool schema, description, and execute() to match
- Update RPC test to use the new field name
@xinzhuwang-wxz xinzhuwang-wxz requested a review from a team May 19, 2026 13:54
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

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: eed17ff8-4e64-455f-b8c3-560b79bed1ee

📥 Commits

Reviewing files that changed from the base of the PR and between 5dd0883 and 2f0e073.

📒 Files selected for processing (1)
  • app/src-tauri/src/core_process.rs

📝 Walkthrough

Walkthrough

Renames the time-window field from window_days to time_window_days across the RPC request, handler, unit test, and the memory query tool; separately increases the embedded-core readiness polling window from ~4s to ~30s.

Changes

Parameter Rename Across RPC and Tool Boundary

Layer / File(s) Summary
RPC Request Contract and Handler Update
src/openhuman/memory/tree/retrieval/rpc.rs
QueryGlobalRequest struct field renamed from window_days to time_window_days; query_global_rpc now forwards req.time_window_days; unit test updated to use time_window_days: 7.
Tool integration: description, schema, and execution
src/openhuman/tools/impl/memory/tree/query_global.rs
Tool description text and parameters_schema() updated to define and require time_window_days; execute() passes req.time_window_days into retrieval::query_global.

Embedded-core readiness timeout tuning

Layer / File(s) Summary
Tauri core readiness loop
app/src-tauri/src/core_process.rs
Increased readiness polling iterations from 40 to 300 (100ms sleeps) and updated comments to reflect new ~30s stuck timeout for embedded-core startup.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • tinyhumansai/openhuman#2255: Also touches QueryGlobalRequest naming for time_window_days vs window_days and related RPC/schema test updates.

Suggested labels

rust-core, working

Suggested reviewers

  • senamakel

Poem

🐰 I hopped through code to make a change,
A field renamed — not wild, just strange.
From window_days to time_window_days I leapt,
And waited longer while the core slept.
Small tidy hops, the tree now sings.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: renaming window_days to time_window_days in the query_global functionality across the memory-tree module.
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.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 19, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 19, 2026
@senamakel senamakel self-assigned this May 20, 2026
# Conflicts:
#	src/openhuman/memory/tree/retrieval/rpc.rs
The 40 × 100ms = 4s loop was the failure timeout for the embedded core
spawn (axum bind + ready signal). On cold CI Docker hardware that's not
enough — ensure_running_falls_back_* tests (and the two that share the
env_lock and cascade with PoisonError) were timing out on every Tauri
Shell CI run, including on main.

Bumped to 300 × 100ms = 30s. The loop short-circuits on ready, so this
only widens the failure-to-fail window, not the success path.
@coderabbitai coderabbitai Bot added rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. working A PR that is being worked on by the team. labels May 20, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 20, 2026
# Conflicts:
#	app/src-tauri/src/core_process.rs
@senamakel senamakel merged commit aaba2ca into tinyhumansai:main May 20, 2026
26 checks passed
@senamakel
Copy link
Copy Markdown
Member

huge thanks @xinzhuwang-wxz, nailing that window_daystime_window_days rename so deepseek (and friends) stop choking on the schema is such a clean catch 🙌. love seeing you back in here, this one's gonna save folks a lot of confused debugging.

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

Labels

rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants