⚡ Fix N+1 Query in Evidence Scope Validation - #73
Conversation
Replaces the iterative per-evidence-ID `query_row` check in `store_review` with a single SQLite bulk query using `json_each` and `EXCEPT`. Measured improvement: - Validating 1000 IDs dropped from ~17.4ms to ~1.07ms (over 15x speedup on happy path). - Added `store_review_evidence_nplus1` benchmark to track both happy and failure paths. Co-authored-by: undivisible <136312656+undivisible@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 016863a252
ℹ️ 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".
| "SELECT value FROM json_each(?1) \ | ||
| EXCEPT \ | ||
| SELECT id FROM evidence WHERE tenant_id = ?2 AND person_id = ?3 AND deleted_at IS NULL", |
There was a problem hiding this comment.
When a person has far more stored evidence than the review cites, the right-hand side of this EXCEPT enumerates every live evidence row in that tenant/person scope and builds a temporary set, so even a review containing one ID now takes time proportional to the person's entire evidence history. The existing evidence_scope index only narrows this to the scope; it does not constrain the scan to the requested IDs. Drive the query from json_each and use a correlated NOT EXISTS or join against the evidence.id primary key, optionally stopping after the first missing row.
Useful? React with 👍 / 👎.
Replaces the iterative per-evidence-ID `query_row` check in `store_review` with a single SQLite bulk query using `json_each` and `EXCEPT`. Measured improvement: - Validating 1000 IDs dropped from ~17.4ms to ~1.07ms (over 15x speedup on happy path). - Added `store_review_evidence_nplus1` benchmark to track both happy and failure paths. Co-authored-by: undivisible <136312656+undivisible@users.noreply.github.com>
Replaces the iterative per-evidence-ID `query_row` check in `store_review` with a single SQLite bulk query using `json_each` and `EXCEPT`. Fixes prettier issue in openclaw. Measured improvement: - Validating 1000 IDs dropped from ~17.4ms to ~1.07ms (over 15x speedup on happy path). - Added `store_review_evidence_nplus1` benchmark to track both happy and failure paths. Co-authored-by: undivisible <136312656+undivisible@users.noreply.github.com>
EXCEPT against all live evidence was O(person evidence). Probe json_each with NOT EXISTS/LIMIT 1 so cost follows the review's evidence_ids.
Replaces the iterative per-evidence-ID `query_row` check in `store_review` with a single SQLite bulk query using `json_each` and `EXCEPT`. Also ignores EPIPE errors in the OpenClaw typescript plugin tests due to Node 24 behavior change. Measured improvement: - Validating 1000 IDs dropped from ~17.4ms to ~1.07ms (over 15x speedup on happy path). - Added `store_review_evidence_nplus1` benchmark to track both happy and failure paths. Co-authored-by: undivisible <136312656+undivisible@users.noreply.github.com>
1 participant
💡 What: Replaced the N+1
query_rowcheck insrc/store/lifecycle.rs:768with a single SQLite query usingjson_eachandEXCEPT.🎯 Why: Validating a list of evidence IDs was previously doing an individual
EXCEPTdatabase query per ID, leading to N+1 performance degradation when reviews contained a large list of evidence.📊 Measured Improvement:
PR created automatically by Jules for task 11381485382194887014 started by @undivisible
Note
Low Risk
Validation semantics stay the same (scoped, non-deleted evidence only); risk is limited to subtle differences in which missing ID is reported when several are invalid.
Overview
store_reviewno longer runs oneEXISTSquery per evidence ID. It serializes the review’sevidence_idsto JSON and uses a single cached query (json_eachEXCEPT liveevidencerows for the tenant/person) to detect any missing or soft-deleted IDs, then returns the sameevidence {id} is unavailableerror on the first missing result.A new
store_review_evidence_nplus1bench (registered inCargo.toml) compares the old per-ID loop vs the batch query on ~1000 IDs for happy path and one-missing cases.schema.rsonly reorders imports.Reviewed by Cursor Bugbot for commit 016863a. Configure here.