Repository navigation
Conversation
GZipMiddleware (app/main.py) compresses any streaming response whose content type Starlette does not exempt, and it exempts only text/event-stream. Its GZipResponder writes each chunk into a GzipFile without flushing, so deflate held every token until the generator closed and the answer arrived as one block. Measured against the real middleware stack, 30 records yielded 50ms apart, lag = arrival - yield: with GZip, Accept-Encoding: gzip mean 0.795s max 1.593s with GZip, Accept-Encoding: identity mean 0.000s max 0.001s without GZip, Accept-Encoding: gzip mean 0.000s max 0.001s BaseHTTPMiddleware does not buffer, so GZip alone is the cause and the fix is scoped to it. Browsers cannot opt out: Accept-Encoding is a forbidden header name. Confirmed on a live server - content-encoding: identity on POST /api/query/stream. This silently defeated the 15s keepalive here and the 50ms flush throttle in useChatStream.ts. Two tests. No existing test could have caught this: conftest builds its client with httpx ASGITransport, which awaits the app to completion and accumulates the body before constructing the Response, so nothing routed through that fixture can observe streaming at all. The timing test therefore drives the ASGI app directly. It asserts a *decodable* record rather than a non-empty body, because gzip emits a 10-byte header on its first write and a non-empty check passes even when fully buffered.
Two SQLite defects, kept in one commit because both live in db.py.
1. idx_chunks_text_lookup ON chunks(id, text_preview) duplicated the
whole compressed corpus: `id` is the rowid, so the index stored a
second full copy of the text column. Nine lines below its
declaration, schema.sql already carried a tombstone for
idx_chunks_covering, dropped in Phase 9 for exactly this reason - the
same defect had been reintroduced under a second name.
A/B on 21,584 chunks of 512-char zlib bodies:
insert 0.072s -> 0.114s (+57.8%)
on disk 4792 KiB -> 9112 KiB (+90.2%)
No read benefit on any shape tested, including the FTS-rebuild
projection, the only query the planner chose it for: 0.0815s with it
against 0.0803s without.
The DROP in _migrate is the load-bearing half. executescript(schema.sql)
runs before _migrate, so the drop wins regardless of what schema.sql
declares - verified by restoring the CREATE and watching the test still
pass. Removing the CREATE only stops every startup building the index
and immediately dropping it again.
2. The seed step of every graph traversal filtered on
json_extract(properties, '$.chunk_id'), which is not sargable, so it
scanned kg_nodes and parsed JSON per row - while kg_nodes.chunk_id sat
there, written on every insert and indexed by nothing.
64,752 nodes, 20 seeds:
json_extract (shipped) 22.54 ms SCAN kg_nodes
chunk_id column, no index 7.04 ms SCAN kg_nodes
chunk_id column + index 0.06 ms SEARCH USING INDEX
All four read sites move to the column; the migration back-fill at
db.py:513 keeps reading JSON, as it must.
Testing the equivalence found a hole: the existing rebuild migration
back-fills chunk_id only when the column is *absent*, so a row whose
provenance lived only in the JSON would have vanished from graph
seeds. Confirmed divergent - with one such row the old predicate
returned it and the column predicate did not. Now back-filled,
skipping rows whose chunk is gone, because chunk_id is a FOREIGN KEY
and adopting a deleted chunk would fail the migration.
test_coverage_booster_gen3 inserted kg_nodes without chunk_id, a row
shape production never writes. It now writes the column, and gained the
chunk row that the FOREIGN KEY needs - a constraint that was simply
unenforced while the column was NULL.
Three endpoints were each cached under two keys - getFileTree as
'file-tree' and 'files-tree', getLLMPreferences as 'llm-preferences' and
'llm-prefs', getOcrStatus as 'ocr-status' and 'ocr-status-settings'. Each
split meant two fetches of one payload and invalidation that never
crossed: removing a folder index in Explorer invalidated only its own
spelling, so Search kept offering the deleted folder until a reload.
cacheKeys.ts now holds one key per endpoint, with a test asserting no two
collide.
invalidateCache() with no argument ran queryClient.clear(), which
*removes* every query rather than marking it stale, so every mounted
observer dropped to data: undefined and every page fell back to its
cold-start spinner. Four call sites did that, including the one firing
when an index run completes - finishing a scan blanked the whole app. The
prefix is now required and the clear() path is gone; those four became
invalidateCorpusCaches(), which touches only the six corpus-derived keys.
An index run cannot change which providers are configured.
Two render defects, both from useApi reporting `isLoading || isFetching`
with refetchOnWindowFocus enabled:
- ExplorerPage replaced the whole tree with a spinner on any background
refetch; now guarded on `&& !tree`, the pattern already used correctly
in four other places.
- SetupPage gated its "Incompatible Storage Detected" warning on
!isLoading, and registers focus/visibilitychange listeners that
refetch - so a storage-compatibility warning blinked out exactly when
the user alt-tabbed back. Both banners now gate on data presence;
isDriveConfigSafe already defaults safe while driveInfo is absent.
Five writes move to useMutation. useOptimisticMutation carries the
ceremony that is easy to get wrong once, let alone five times: cancel
in-flight refetches so a stale response cannot land on top of the
optimistic value, snapshot, roll back on error, re-sync either way.
- remove folder: leaves the tree on click; the button had no pending
state at all, so a double-click sent two requests
- OCR enabled and cloud-privacy consent checkboxes: both bound `checked`
straight to server data and visibly snapped back until the refetch
- fallback-chain reorder: shares the provider-settings cache entry
- Clear Index and Refresh: no optimistic state to show, but no pending
state either until now
Repointing the three stale mocks revealed they had been returning
undefined and nothing asserted on it. With ocr-status live, OcrSection
renders and calls getOcrQueue, which a bare vi.fn() resolves as
undefined - hence the mock and act() fixes in SettingsPage.test.tsx. The
InsightsPage act() fix is unrelated to this change and pre-existing: 2
warnings at HEAD, 0 with it.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 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 |
b89bc10 ("Merge branch 'main' into updates", the PR's Update-branch button) re-added the pre-616b966 getOcrStatus hook alongside the one 616b966 had just rewritten. 616b966 replaced the 'ocr-status' string literal with CACHE_KEYS.ocrStatus; main still carried the literal, and the two landed at non-overlapping offsets, so git merged them clean instead of raising a conflict. The result declared `ocr` twice and esbuild refused the file, taking LibraryPage.test.tsx out of collection with "The symbol ocr has already been declared". Deletes the resurrected block. LibraryPage.tsx is now byte-identical to its 616b966 version.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.