fix(ts-client): prune out-of-scope batch resources - #3085
fix(ts-client): prune out-of-scope batch resources#3085OldFriendWenjianjian wants to merge 1 commit into
Conversation
WalkthroughBatch cache updates now prune out-of-scope resource IDs before merging matching resources. The logic supports direct arrays and wrapped responses. Tests cover mixed batches, cache states, metadata replacement, and case-sensitive POSIX paths. ChangesScoped cache updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/ts-client/tests/useNormalizedQuery.mixed-batch.test.ts (1)
65-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTraced expected outputs against the source logic; all four
updateCache/updateBatchResourcesscenarios match.Verified step by step: the Windows-path mixed batch (lines 66-97), the direct-array mixed batch (lines 99-120), the full-move-out batch (lines 122-136), and the full-in-scope batch (lines 138-150) all produce the array contents and merged field values the assertions expect, given
filterBatchResources,removeResourcesFromCacheData,updateArrayCache, andupdateWrappedCacheinuseNormalizedQuery.ts.One coverage gap: none of these tests exercise the
oldDatabeingundefined(the "query hasn't returned yet" seed path described in the comment atuseNormalizedQuery.tslines 584-587) combined with a mixed batch. Consider adding a test that callsupdateBatchResourceswithout first callingqueryClient.setQueryData, with a batch containing both in-scope and out-of-scope resources, to confirm the cache seeds only with the in-scope subset.🤖 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 `@packages/ts-client/tests/useNormalizedQuery.mixed-batch.test.ts` around lines 65 - 162, Add a test in the updateBatchResources scoped-batches suite that invokes updateBatchResources without seeding queryClient data, using a batch containing both in-scope and out-of-scope resources. Assert the resulting direct array cache is initialized only with the in-scope resources, covering the oldData-undefined path in updateArrayCache.
🤖 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.
Inline comments:
In `@packages/ts-client/src/hooks/useNormalizedQuery.ts`:
- Around line 576-644: Update removeResourcesFromCacheData so wrapped cache
responses recompute total_count from the filtered arrayField and keep has_more
consistent with the pruned result, including when all resources are removed.
Preserve unrelated metadata and existing array-response behavior, and add a
regression assertion in the mixed-batch tests covering partial or full pruning.
- Around line 617-644: Update the batch-cache pruning flow around
updateBatchResources so bare single-resource objects are removed when their id
is in the outOfScopeResourceIds batch. Reuse the same oldData.id deletion
behavior as deleteResource before updateWrappedCache resolves filteredResources,
while leaving removeResourcesFromCacheData unchanged for non-batch
single-resource events.
---
Nitpick comments:
In `@packages/ts-client/tests/useNormalizedQuery.mixed-batch.test.ts`:
- Around line 65-162: Add a test in the updateBatchResources scoped-batches
suite that invokes updateBatchResources without seeding queryClient data, using
a batch containing both in-scope and out-of-scope resources. Assert the
resulting direct array cache is initialized only with the in-scope resources,
covering the oldData-undefined path in updateArrayCache.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 16630db1-205d-4230-b590-2a15a834283a
📒 Files selected for processing (2)
packages/ts-client/src/hooks/useNormalizedQuery.tspackages/ts-client/tests/useNormalizedQuery.mixed-batch.test.ts
1 participant
Summary
A scoped
useNormalizedQuerysubscription receives an entire atomicResourceChangedBatchwhen any resource in the batch matches its path scope. Previously, when a batch mixed resources that remained in the current directory with resources that moved out, the hook merged only the in-scope resources but did not remove the moved-out IDs. This could leave ghost entries in the old directory after a bulk move.This change updates the cache in one transaction: it removes IDs for resources filtered out of the current scope, then merges the resources that still match. Existing all-in-scope and all-out-of-scope behavior is preserved.
Validation
bun test packages/ts-client/tests/useNormalizedQuery.mixed-batch.test.ts: 5 passed, 0 failedbun run --filter @sd/tauri build: passedbun run --filter @sd/interface typecheck: existing mainline baseline failure (846 TypeScript error lines); changed-file diagnostics: 0bun run --filter @sd/ts-client build: existing mainline failure inuseSearchFiles.ts:100becauseSearchFiltersfields are missing; no diagnostic in the changed production fileThe focused tests cover mixed batches for wrapped and direct-array caches, all resources moving out, all resources remaining in scope, Windows separator/case normalization, and POSIX case-sensitive matching.
Manual Explorer smoke testing was not run.
Note
Summary: Fixes a cache pruning bug in
useNormalizedQuerywhere atomic batches with mixed resources (some in scope, some moved out) would fail to remove out-of-scope IDs, leaving ghost entries after bulk operations. The fix atomically removes out-of-scope resource IDs before merging in-scope updates.Changes: Added
removeResourcesFromCacheData()helper that filters out IDs for resources outside the query scope before cache updates. Both array and wrapped cache responses are handled.Testing: New test suite covers mixed-batch scenarios including case-insensitive path matching on Windows, POSIX case-sensitive matching, and both wrapped (
{ files: [...] }) and direct array responses.Written by Tembo for commit a7498a3c3. This will update automatically on new commits.