fix(state): harden maintenance findings#98
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces a "core memory" system for project maintenance, replacing dated AI review reports with a centralized core-memory.md and updating documentation across the repository. The state platform's query service was modified to handle sparse matches by implementing a scanning loop that continues until the requested limit is met, and the SDK was refactored to formally integrate with LangChain's BaseCheckpointSaver. Feedback identifies high-severity risks regarding the new scanning logic, specifically the potential for unbounded execution times and Worker timeouts during extremely sparse queries. Additionally, the reviewer suggested optimizing the row filtering within these loops to reduce redundant processing overhead.
| let cursor = Number(input.cursor); | ||
| const pageSize = Math.max(limit * 5, limit); | ||
|
|
||
| while (rows.length < limit) { |
There was a problem hiding this comment.
This while loop implements a scan that continues until the limit is satisfied. While this fixes the issue with sparse matches, it introduces a risk of unbounded execution time. If a query is extremely sparse (e.g., matching 1 row out of 100,000), this could trigger hundreds of sequential D1 queries, likely exceeding the Cloudflare Worker CPU time limit or D1's request limits.
Consider adding a safety limit on the number of iterations or the total number of rows scanned. If the limit is reached, return the results found so far with a next_cursor pointing to the next sequence to scan, allowing the client to continue in a subsequent request.
| let cursor = Number(input.cursor); | ||
| const pageSize = Math.max(limit * 5, limit); | ||
|
|
||
| while (rows.length < limit) { |
| updated_at: row.created_at, | ||
| deleted_at: null, | ||
| }; | ||
| if (!filterStateRows([state], input).length) continue; |
There was a problem hiding this comment.
Calling filterStateRows([state], input) inside the loop is inefficient. It creates a new array and re-allocates/re-processes the query predicates for every single row fetched from the database.
Since this loop is intended to handle potentially many non-matching rows, this overhead adds up quickly. Consider preparing the predicates once outside the loop and calling an optimized matching function directly.
dceaf87 to
1385ec4
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Co-Authored-By: Duyet Le <me@duyet.net> Co-Authored-By: duyetbot <bot@duyet.net>
1385ec4 to
e68b3aa
Compare
|
Addressed Gemini review: added a per-request sparse-query scan cap, returns a continuation cursor when the cap is reached, and precomputes predicates instead of rebuilding them per row. Re-verified locally with API typecheck, Biome, focused state-platform test, and full API tests (307 passed). @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e68b3aa2dd
ℹ️ 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".
| return { | ||
| rows, | ||
| nextCursor: Number.isFinite(cursor) && cursor > 0 ? String(cursor) : null, | ||
| }; |
There was a problem hiding this comment.
Follow continuation cursors even for empty sparse pages
This new return path can emit a non-null nextCursor with rows still empty when the scan cap is reached, but AgentStateCheckpointSaver.queryRecords currently stops pagination on rows.length === 0 (packages/sdk/src/langgraph.ts:338). In sparse datasets, that means the SDK will terminate early and miss valid later matches even though the API explicitly provides a continuation cursor (as covered by the new sparse-cap test), so LangGraph checkpoint queries can become incomplete.
Useful? React with 👍 / 👎.
| "peerDependencies": { | ||
| "@langchain/langgraph-checkpoint": "^1.0.0" | ||
| "@langchain/langgraph-checkpoint": "^1.0.2" | ||
| }, |
There was a problem hiding this comment.
Declare @langchain/core as a peer for the langgraph entry
The commit adds a direct import from @langchain/core/runnables in packages/sdk/src/langgraph.ts, but package.json still exposes only @langchain/langgraph-checkpoint as a peer. In environments that do not auto-install transitive peers, importing @agentstate/sdk/langgraph can fail with a module-resolution error for @langchain/core, so the SDK’s runtime dependency contract is now incomplete.
Useful? React with 👍 / 👎.
Summary
/api/v2/states/querypagination so tag/JSON-path filters keep scanning past nonmatching rowsBaseCheckpointSaverand validate that contract in SDK buildsEvidence
cbc5df6throughc957835packages/api/src/services/states.ts, predicate mutation inmatchesPredicates, local empty LangGraph base class inpackages/sdk/src/langgraph.tsdocs/API_DOCS_REVIEW.md,docs/test-coverage-analysis.mdVerification
bun install --frozen-lockfilebunx biome check packages/*/src/bunx tsc --noEmit -p packages/api/tsconfig.jsoncd packages/dashboard && bunx tsc --noEmitcd packages/api && bunx vitest run(306 passed)cd packages/api && bunx vitest run test/state-platform.test.ts(7 passed)cd packages/sdk && bun run typecheckcd packages/sdk && bun run test(11 passed)cd packages/sdk && bun run buildbun run test:sdk-examplesbun run build