Supabase switchover readiness: tests, parity fixes, mixed-mode auth, CI fixture#621
Draft
hatton wants to merge 49 commits into
Draft
Supabase switchover readiness: tests, parity fixes, mixed-mode auth, CI fixture#621hatton wants to merge 49 commits into
hatton wants to merge 49 commits into
Conversation
…e + Blorg milestone)
Anonymous browsing (grids, search, book detail, language/topic menus) can now run
against a local Supabase (see bloom-core-supabase repo): select with
VITE_DATA_LAYER_IMPL=supabase. Parse remains the default.
- Supabase{Book,Language,Tag}Repository implement the existing data-layer
interfaces; user/auth/analytics arms are anon-safe stubs
- SupabaseBookQueryBuilder translates IFilter to PostgREST (guards, tags,
language via lang_pointers, features, derivedFrom, anyOfThese, pagination,
ordering); known v0 divergences documented in
src/data-layer/implementations/supabase/README.md
- SupabaseBookMapper converts rows back to the Parse shape so
createBookFromParseServerData keeps producing Book objects unchanged
- Integration test (RUN_SUPABASE_TESTS=true) against the local stack
- Verified end-to-end in the browser: home page (112 books), language grids,
book detail incl. uploader + S3 thumbnails, title search
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tests; document local Supabase setup - bookHash:, phash:, copyright:, country:, publisher:, originalPublisher:, edition:, branding(ProjectName):, license:, harvestState: facets now translate to PostgREST with Parse-matching semantics (a dropped facet made /bookHash:... return every book) - Integration tests now assert that facets constrain results and that the 'Other' topic exclusion works (regression guards for dropped-filter and PostgREST array-literal bugs) - README: Supabase section with one-time Windows/Podman setup, per-session startup, and test instructions Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…EADME - Wildcard tags (bookshelf:X*, *suffix) now match per array element through the generated tags_text column (companion migration in bloom-core-supabase); any-of wildcard lists fail closed instead of dropping the constraint - Integration test asserting wildcard prefixes constrain correctly - README: bloom-core-supabase now lives at D:\bloom-core-supabase Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Synthesized from a 3-way gap analysis (data-layer parity matrix, whole-repo Parse-usage sweep, bloom-core-supabase backend inventory). Defines what "100% ready to switch" means for the anonymous-browsing scope and tracks each gap to closure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
getBloomApiHeaders() read the session token from connection/ParseServerConnection's singleton, but nothing populates that since login moved to the data-layer's ParseAuthenticationService (which stores the token in its own ParseConnection). Every authenticated Bloom API call was sending Authentication-Token: undefined (useGetPermissions, deleteBook, ParseUserRepository). Now resolves the active authentication service at call time and omits the header when there is no session, which also gives correct anonymous behavior under the Supabase data layer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
69 CI-safe tests (no local stack needed) using a recording fake of the postgrest-js builder: guard rails (in_circulation/draft/rebrand), free- text ilike translation, every search facet, wildcard/canonical/Other topic handling, derivedFrom/anyOfThese client-side unioning with fail- closed paths, all ordering schemes, and full row->model mapping incl. null tolerance. Documents (test + NOTE, no source change) that the "edition:" facet is dead code: it is missing from the shared facets list in connection/BookQueryBuilder.ts, so it never reaches either builder. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes connection/ParseServerConnection.ts (nothing imported its login/logout/moderator/concern-email functions since auth moved to ParseAuthenticationService) and connection/LibraryUpdates.ts (its free updateBook had no callers; the repository method is what the app uses). The live anonymous parse-server queries in LibraryQueries.ts and LibraryQueryHooks.ts now use the data-layer ParseConnection singleton - one source of truth for URL/app-id, and authenticated reads now carry the session token, which the legacy singleton never had populated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
12 more gated tests (RUN_SUPABASE_TESTS=true) against the local stack: publisher/level/feature facets, otherTags exact+wildcard, language+ topic combination, anyOfThese union with dedup verification, derivedFrom negation, count consistency, pagination disjointness, and scale-timing probes. Against a 699-book local dataset: broad search ~150ms, anyOfThese ~120ms - the client-side ID-unioning risk (A5) looks fine at this scale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iod) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hatton
commented
Jul 18, 2026
Member
Author
|
[Claude Fable 5] Consulted Devin on 2026-07-18 up to commit f8ffc08 (review ran against e579483; re-review of f8ffc08 in progress). It raised 3 bugs and 5 investigate flags, all write-path regressions from the earlier data-layer refactor — each has its own review thread above with the outcome (7 fixed in f8ffc08, 1 assessed not-a-bug). 1 informational item (bulk-edit limit under Supabase uses range()) was reviewed, no action needed. CI (supabase-integration) green. |
Two more Devin re-review findings verified real: - SupabaseBookRepository.rowToBasicBookInfo never populated lang1Tag (Parse derives it from show.pdf.langTag); book cards in the anonymous browsing path now get it from the same field, with unit tests. - The BookCount rewrite lost master's stale-count protection: a reused BookCountInternal briefly showed the previous filter's count, and the old hasStarted ref both updated state after unmount (the React warning seen in the smoke test) and could skip a mid-flight filter change. useGetBookCountWithError now cancels per-effect, tracks the last-loaded filter synchronously, and reports loading until the current filter's count arrives. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hatton
commented
Jul 18, 2026
hatton
commented
Jul 18, 2026
hatton
commented
Jul 18, 2026
hatton
commented
Jul 18, 2026
hatton
commented
Jul 18, 2026
Member
Author
|
[Claude Fable 5] Consulted Devin (round 2) on 2026-07-18 up to commit 78f48bb. Its re-review of f8ffc08 confirmed all three round-1 bugs fixed and raised 5 new items — threads above: 2 confirmed and fixed in 78f48bb (Supabase lang1Tag on book cards; BookCount stale-count protection), 3 assessed not-an-issue with reasoning. A round-3 re-review of 78f48bb is running; CI remains green. |
Book instances strip "level:X" out of tags into book.level on load, and the bulk Add Tag flow wrote the stripped array back wholesale, so every bulk tag addition permanently erased the books' reading levels (a regression vs master's raw-record flow; saveAdminData already re-adds the level for the same reason). The bulk path now re-merges level:<book.level> before writing. Tests verified to fail pre-fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hatton
commented
Jul 18, 2026
hatton
commented
Jul 18, 2026
Member
Author
|
[Claude Fable 5] Consulted Devin (round 3) on 2026-07-18 up to commit 0064746. The re-review of 78f48bb confirmed the round-1 bugs fixed (marked resolved by Devin) and raised 2 new items, threaded above: the bulk-tag reading-level deletion (confirmed, fixed in 0064746) and the GridExport stats join (verified not an issue). A final re-review of 0064746 is running; CI green throughout. |
Extracted-tag stripping (level: -> book.level) made book.tags a lossy view, and every write site had to remember to re-merge - the mechanism behind the bulk-edit level-deletion bug. Book.ts now owns a matched pair: extractedTagPrefixes drives updateTagsFromParseServerData, and the new getTagsForSaving() re-merges every extracted field; both saveAdminData and the bulk Add Tag path use it, and a round-trip test parameterized over the prefix list fails loudly if a future extracted field forgets its merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The book === null branch ran before the error branch, and a failed load leaves book null, so transient failures showed the misleading not-found message (BookDetail.tsx already ordered these correctly). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hatton
commented
Jul 18, 2026
Member
Author
|
[Claude Fable 5] Consulted Devin (round 4) on 2026-07-18 up to commit 89d7312. Its re-review of 0064746 marked all four previously-found bugs resolved and raised one new item (threaded above, fixed in 89d7312). This push also includes the approved tags-serialization hardening (Book.getTagsForSaving paired with the tag extractor + round-trip invariant test) so future extracted-tag fields cannot silently reintroduce the bulk-edit data-loss class of bug. Final re-review of 89d7312 running; CI green throughout. |
hatton
commented
Jul 18, 2026
hatton
commented
Jul 18, 2026
Member
Author
|
[Claude Fable 5] Consulted Devin (round 5, final) on 2026-07-18 up to commit 89d7312. It confirmed all five previously-found bugs resolved. Two remaining items are escalated to the developer and their threads above stay open: silent failed-save errors in admin screens, and book writes throwing under the Supabase flag (a mixed-mode scope question). CI green at every commit. |
…low into data-layer Semantic resolutions beyond the textual conflicts: - Ported master's ParseServerConnection.ts changes into the data-layer's ParseAuthenticationService (which replaced it on this branch): the BL-16516 photoUrl pass-through to the editor login POST, and the BL-14503 fail-fast when Parse returns no sessionToken or email. connectUser() now takes (jwtToken, emailAddress, photoUrl?). - Deleted src/connection/ParseServerConnection.ts (superseded by the data-layer) and master's new ParseConnectionConfig.ts (its only consumer was the deleted file; the data-layer's ParseConnection already serves that role). - package.json: master's pinned-version pnpm layout plus this branch's @supabase/supabase-js and @playwright/test. supabase-js pinned to 2.110.2 because master's minimumReleaseAge supply-chain guard (7 days) rejects 2.110.7 (published 2026-07-16). - tsconfig: master's moduleResolution "bundler" plus this branch's FilterTypes path alias, rewritten without the TS-6-deprecated baseUrl. - Fixed the 10 pre-existing tsc errors master carries in Routes.tsx and ContentfulImage.tsx (master's CI never runs bare tsc; this branch's gate does). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… log C5 The merge deleted ParseConnectionConfig.ts as orphaned, but master's new book-navigation-interceptor service worker (a .js file) imports it — the vite build caught it. Restored verbatim from master. That service worker does anonymous Parse REST reads of its own, outside the data layer, so it is a new switchover blocker: recorded as C5 in SWITCHOVER-READINESS.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The legacy connection layer (LibraryUpdates.updateBook) caught failed saves and showed alert(error); the data-layer rewrite dropped that, and every call site of Book.saveAdminData / saveArtifactVisibility is fire-and-forget, so a moderator whose edit failed to save was never told. Catch at the Book method level, console.error the underlying error, and show the same alert the legacy path showed. Unit tests mock a rejecting repository and assert the alert fires and nothing rejects. Addresses the Devin review thread r3609217139 on PR #621. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Under VITE_DATA_LAYER_IMPL=supabase, book writes previously routed into SupabaseBookRepository's unimplemented write methods and threw. The Supabase impl key now registers HybridBookRepository: the nine read methods delegate to SupabaseBookRepository, and the three write methods (updateBook, deleteBook, saveArtifactVisibility) delegate to ParseBookRepository — mirroring the auth/user mixed-mode registration, and authenticated by the same Parse session token. Delegation tests cover every interface method; the factory test asserts the hybrid is what the Supabase impl serves. Addresses the Devin review thread r3609217609 on PR #621. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The yarn setup-node cache and yarn install steps broke once the merge adopted master's pnpm migration (corepack rejects the packageManager field under yarn 1.x). Use the same setup-vp action as build-and-deploy.yml. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-cap gap The title-cleanup regex /[\r\n\v]+/g matched literal backslashes and the letters r/n/v instead of collapsing line breaks (no live caller today — the read path builds Book, not BookModel — but wrong as written). Now matches the real control characters, same as Book.getBestBookTitle. Also recorded readiness item D4: under the Supabase read path, bulk-edit reads are clamped to PostgREST max_rows (1000), so a moderator bulk operation over more matches would silently process only the first page — accepted as a moderator-scope known gap on the same basis as D3, needs paged reads in a Supabase moderator milestone. And refreshed D3's wording now that HybridBookRepository delegates writes to Parse instead of throwing. Both flagged by Devin round 6 on PR #621. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hatton
commented
Jul 19, 2026
hatton
commented
Jul 19, 2026
hatton
commented
Jul 19, 2026
Member
Author
|
[Claude Fable 5] Consulted Devin (rounds 6–7) up to Round 6 re-reviewed the whole PR after the master merge: 6 of its 8 bugs it now marks resolved (including the failed-save alerts fixed today), and the rest were repeats already documented in resolved threads — except two genuinely new findings, each with its own thread: a doubly-escaped regex in the new BookModel's title cleanup (fixed in d002ade) and bulk-edit reads being clamped by PostgREST's 1000-row cap under Supabase (accepted as moderator-scope readiness item D4). Round 7 on d002ade confirmed the regex fix and raised one new flag — the standalone artifact-visibility control can't save through the read-only detail-hook shim — accepted with reasoning in its thread (Storybook-only path). 11 informational flags reviewed, none actionable. CI (supabase-integration) green on every push. All 21 Devin threads on this PR are now resolved with documented outcomes; nothing is left waiting on a decision. |
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
1 participant
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.
Proves blorg is ready to flip
VITE_DATA_LAYER_IMPL=supabasefor the anonymous-browsing scope, per the readiness checklist added asSWITCHOVER-READINESS.md(all A/C/D items closed or explicitly accepted; B4 CI contract tests included here).Highlights:
SupabaseBookQueryBuilder/SupabaseBookMapper; 23 gated integration tests; scrubbed 699-book SQL fixture + CI workflow running the integration suite.ParseServerConnection.ts,LibraryUpdates.ts);freeLearningIOrouted through the repository; unreachableedition:facet branches removed.topic:filters resolved via the newmatch_topic_tagsRPC (bloom-core-supabase PR language links in book detail #9).Known state: NOT yet integrated with master (66 new commits there incl. yarn→pnpm and TS 6.0.3 migrations plus login-flow changes conflicting with this branch's auth rewiring) — integration is a tracked follow-up decision.
🤖 Generated with Claude Code
This change is