From 0a1fdb5aa0a81131565d296bafea0f01a4fcc873 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 30 Sep 2026 13:06:46 +0200 Subject: [PATCH] docs(11-05): complete beachcomber search sync plan --- .../11-05-SUMMARY.md | 281 ++++++++++++++++++ .../deferred-items.md | 11 + 2 files changed, 292 insertions(+) create mode 100644 .planning/phases/11-jobs-realtime-and-search-infrastructure/11-05-SUMMARY.md create mode 100644 .planning/phases/11-jobs-realtime-and-search-infrastructure/deferred-items.md diff --git a/.planning/phases/11-jobs-realtime-and-search-infrastructure/11-05-SUMMARY.md b/.planning/phases/11-jobs-realtime-and-search-infrastructure/11-05-SUMMARY.md new file mode 100644 index 0000000..fb26636 --- /dev/null +++ b/.planning/phases/11-jobs-realtime-and-search-infrastructure/11-05-SUMMARY.md @@ -0,0 +1,281 @@ +--- +phase: 11-jobs-realtime-and-search-infrastructure +plan: 05 +subsystem: search +tags: [typesense, search, gorm-callbacks, after-commit, kill-switch, scout] + +requires: + - phase: 11-jobs-realtime-and-search-infrastructure + provides: "11-01 lagoon.OnDatabase, lagoon.Transaction and lagoon.AfterCommit (with the lagoon:after_commit flush)" + - phase: 11-jobs-realtime-and-search-infrastructure + provides: "11-03 lighthouse init-time driver registry and per-*gorm.DB callback installation idiom" +provides: + - "modules/beachcomber: Searchable, IndexSchemaProvider, SearchKeyer, Engine, Query, Gate/GateFunc, RegisterEngine, the null engine, Service (From, SetGate, Engine, Prefix, IndexName, Sync, Remove)" + - "GORM callbacks beachcomber:after_create/after_update/after_delete pinned between the model's after hook and GORM's commit, registering an after-commit, gated, reload-then-upsert-or-delete sync" + - "modules/beachcomber/typesense: hand-rolled net/http engine on the Scout wire contract (X-TYPESENSE-API-KEY, collection create on 404, JSONL import with success:false scan, 404-tolerant delete/flush, SearchIDs), StatusError, Config from search.typesense.*" + - "fonoteka.go: Album Searchable binding (PHP toSearchableArray field for field, AlbumMedia, MediumFamily, collection schema, AlbumSearchQueryBy), settingsGate on search_use_typesense, wireSearch in Boot, config/search.yaml, README env mapping" +affects: [11-07 unit tests, 12 album search endpoint (SQL re-gate of SearchIDs), 13 CSV import (bulk reindex via Service.Sync), 15 cutover env mapping] + +actuals: + tokens: 19835 + tasks: 2 + commits: 2 +plan_head_before: 0d45698ad7d25e73e369c9ffcdf697c356f45bbf +plan_head_after: 9543e6508c74d1c28f8b1f264c933ba82ec7603e +# fonoteka.go (separate repository) received 3 more commits: c4a7449, 3b32946, 0ce04ab + +tech-stack: + added: [] + patterns: + - "Search engines register from init (beachcomber.RegisterEngine) and are chosen by search.driver, like lighthouse drivers" + - "Side-effect GORM callbacks that use lagoon.AfterCommit must be pinned with Before(\"gorm:commit_or_rollback_transaction\"); an After-only anchor lands past the commit" + - "Code running on a callback's handle builds a clean session (Session NewDB+Context, Clauses(), Session NewDB) before handing it to application code" + - "Models implement framework contracts structurally, so the models package stays a leaf; the plugin package holds the compile-time assertion" + +key-files: + created: + - modules/beachcomber/beachcomber.go + - modules/beachcomber/searchable.go + - modules/beachcomber/sync.go + - modules/beachcomber/engines.go + - modules/beachcomber/README.md + - modules/beachcomber/typesense/config.go + - modules/beachcomber/typesense/engine.go + - ../fonoteka.go/config/search.yaml + - ../fonoteka.go/plugins/golem15/fonoteka/search.go + - ../fonoteka.go/plugins/golem15/fonoteka/models/album_search.go + - ../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go + - .planning/phases/11-jobs-realtime-and-search-infrastructure/deferred-items.md + modified: + - README.md + - ../fonoteka.go/README.md + - ../fonoteka.go/plugins/golem15/fonoteka/plugin.go + +key-decisions: + - "beachcomber sync callbacks are registered After(gorm:after_*) AND Before(gorm:commit_or_rollback_transaction): GORM appends an After-only callback to the end of the chain, past the commit and past lagoon:after_commit, so single-statement writes never synced" + - "The gate and ToSearchableArray receive a clean session: gorm Session{NewDB, Context} clones the write's statement and a later WithContext queried through the Album table (the gate read an album row as the settings row)" + - "Sync reads run in a savepoint when the handle is inside a caller's plain transaction, so a failed gate or reload read (for example no settings table yet) never aborts the caller's transaction" + - "Engine calls run on context.WithoutCancel(ctx): a request cancelled right after commit still indexes, bounded only by the engine timeout" + - "Index name is search.prefix + SearchableAs() as the plan specifies; PHP Album overrides searchableAs without Scout's prefix, so the two differ only when SCOUT_PREFIX is non-empty (default empty)" + - "Typesense errors are typesense.StatusError (method, path, status) and import failures carry the Typesense message capped at 200 bytes, never the document or answer body" + - "A missing index schema creates an auto-typed collection (fields .* auto) instead of Scout's name-only create, which Typesense rejects; 409 on create counts as success" + - "Album.ShouldBeSearchable returns true; the settings Gate is the kill-switch (plan flagged assumption)" + +patterns-established: + - "beachcomber.Service.Sync / Remove: explicit gated sync for reindex and bulk paths" + - "SearchIDs results are candidates only; callers re-gate in SQL" + +requirements-completed: [SRCH-01] + +coverage: + - id: D1 + description: "Toggle on with an API key: an album created in lagoon.Transaction sends nothing before commit, then GET /collections/golem15_fonoteka_albums (404), POST /collections (name, default_sorting_field created_at, 17 fields), POST import?action=upsert (text/plain JSONL) whose document has 17 keys, string id and positive integer collection_id; X-TYPESENSE-API-KEY on every request" + requirement: SRCH-01 + verification: + - kind: integration + ref: "../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go#TestAlbumSearchSmoke" + status: pass + human_judgment: false + - id: D2 + description: "Kill-switch: toggle off (lagoon.Transaction and plain create) and empty api_key send zero requests; the API key never reaches the logs (mutation check: a gate forced on makes the toggle-off subtest fail)" + requirement: SRCH-01 + verification: + - kind: integration + ref: "../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go#TestAlbumSearchSmoke" + status: pass + human_judgment: false + - id: D3 + description: "Soft delete sends DELETE /collections/golem15_fonoteka_albums/documents/{id}; a 404 delete is not a failure; restoring the album re-indexes it" + requirement: SRCH-01 + verification: + - kind: integration + ref: "../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go#TestAlbumSearchDeleteAndFailures" + status: pass + human_judgment: false + - id: D4 + description: "Typesense 500, a success:false import line and a never-answering endpoint (1s timeout) leave the album committed with exactly one Warn naming index and key; the error carries the status, not the body; non-2xx answers expose StatusCode()" + requirement: SRCH-01 + verification: + - kind: integration + ref: "../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go#TestAlbumSearchDeleteAndFailures" + status: pass + human_judgment: false + - id: D5 + description: "Transaction paths: rollback sends nothing, a plain gorm transaction syncs immediately through the tx handle, a single-statement create syncs after its implicit commit" + requirement: SRCH-01 + verification: + - kind: integration + ref: "../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go#TestAlbumSearchDeleteAndFailures" + status: pass + human_judgment: false + - id: D6 + description: "An album with collection_id 0 is refused by ToSearchableArray, logged, and never sent (FK disabled inside a rolled-back transaction to create it)" + requirement: SRCH-01 + verification: + - kind: integration + ref: "../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go#TestAlbumSearchDeleteAndFailures" + status: pass + human_judgment: false + - id: D7 + description: "SearchIDs sends q, query_by, filter_by collection_id:=5, page, per_page with the API key and returns hit ids in order; no hits is an empty non-nil list" + requirement: SRCH-01 + verification: + - kind: integration + ref: "../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go#TestAlbumSearchDeleteAndFailures" + status: pass + human_judgment: false + - id: D8 + description: "Live smoke against typesense/typesense:26.0 with the admin toggle on: save an album and query the collection" + verification: [] + human_judgment: true + rationale: "Manual smoke listed in the plan's verification, collected at /gsd-verify-work; no live Typesense in CI" + +duration: 24min +completed: 2026-09-30 +status: complete +--- + +# Phase 11 Plan 05: beachcomber search sync and the Album Typesense binding Summary + +**A new framework package, `beachcomber`, syncs GORM models to a search index right after the write commits. It uses a hand-rolled Typesense engine on the Scout wire contract and never contacts Typesense while the `search_use_typesense` kill-switch is off or the API key is empty. Płytarium's Album documents match PHP `toSearchableArray` field for field and always carry a positive `collection_id`.** + +## Performance + +- **Duration:** 24 min +- **Started:** 2026-09-30T10:40:56Z +- **Completed:** 2026-09-30T11:05:16Z +- **Tasks:** 2 +- **Files modified:** 14 (8 in summercms.go, 6 in fonoteka.go), plus deferred-items.md + +## Accomplishments + +- **beachcomber core (D-19).** + - The contracts: `Searchable`, `IndexSchemaProvider`, `SearchKeyer`, `Engine`, `Query` and `Gate`/`GateFunc`. + - Engines come from an init-time registry: `RegisterEngine` panics on a duplicate name, and the built-in `null` engine is the default. `From` reads `search.driver` and `search.prefix`; an unknown driver fails boot and lists the registered engines. + - The callbacks are installed through `lagoon.OnDatabase` with Register-or-Replace per `*gorm.DB`. `Sync` and `Remove` run the same gated path on demand for reindex tooling. +- **After-commit sync (D-20).** + - Each Searchable row with a primary key registers work with `lagoon.AfterCommit`. + - The work runs three request-free gates in order: the engine is configured, a database is published, the application Gate is on. + - It then reloads the row with `Unscoped()`. A delete, a missing row, a soft-deleted row or `ShouldBeSearchable` false sends a delete; otherwise the document is upserted. + - Every error, timeout or panic becomes one Warn log with index, key and operation. The write stays committed. +- **Typesense engine.** + - Every request carries `X-TYPESENSE-API-KEY`. + - Upsert reads the collection, creates it from the schema plus `name` on 404, then imports JSON lines with `action=upsert`. Any `success:false` answer line is an error. + - Delete and Flush treat 404 as success. `SearchIDs` returns candidate ids. + - Every request is bounded by the timeout. A non-2xx answer is a `StatusError` that carries no answer body. +- **fonoteka.go.** + - The Album port covers `AlbumMedia`, `MediumFamily`, `SearchableAs`, `ToSearchableArray` (17 PHP keys and types; styles ordered by name, artists by pivot `sort_order`; refuses collection_id 0) and `SearchIndexSchema`. + - `settingsGate` reads `golem15_fonoteka_settings`. Any read error or a missing row counts as off. + - Boot calls `wireSearch`. + - `config/search.yaml` holds the Scout defaults, and the README maps `SCOUT_*` and `TYPESENSE_*` to `SUMMER_SEARCH__*`. + +## Task Commits + +summercms.go: +1. **Task 1: after-commit upsert with collection_id (tracer)**: `3e1f3e6` (feat) +2. **Task 2: deletes, failures, transaction paths, search ids**: `9543e65` (fix: callback ordering, clean session, `StatusError`, README semantics) + +fonoteka.go: +1. **Task 1**: `c4a7449` (feat: Album Searchable, settings gate, wiring, config, `TestAlbumSearchSmoke`) +2. **Task 2**: `3b32946` (test: `TestAlbumSearchDeleteAndFailures`), `0ce04ab` (docs: README env mapping) + +## TDD Gate Compliance (Task 2) + +- **RED:** `TestAlbumSearchDeleteAndFailures` was written against the Task 1 code. It failed on 6 of its 10 subtests with target assertions, not load errors: + - `plain create was not synced: []` + - `no import inside the plain transaction: []` + - `Upsert error … status 500 does not expose status 500` + - soft delete, `success:false` and zero-collection_id warnings missing, because those writes never synced + + The evidence record was checked with `gsd-tools check tdd-red-evidence` and returned `RED_EVIDENCE_OK` (target `TestAlbumSearchDeleteAndFailures`, 15 tests, 7 failing). The go test output was converted to surefire XML for the checker. Tests and code land in one commit, as green-at-every-commit requires. +- **GREEN:** `9543e65` makes all 13 subtests of both tests pass under `-race`. +- **REFACTOR:** none needed. + +## Files Created/Modified + +- `modules/beachcomber/searchable.go`: the model, engine, query and gate contracts +- `modules/beachcomber/engines.go`: engine registry and the `null` engine +- `modules/beachcomber/beachcomber.go`: `Service`, `From`, `SetGate`, `IndexName`, callback installation +- `modules/beachcomber/sync.go`: callbacks, after-commit registration, gates, reload, savepoint, clean session, `Sync`/`Remove` +- `modules/beachcomber/typesense/config.go`, `engine.go`: `search.typesense.*` config and the HTTP engine +- `modules/beachcomber/README.md`, `README.md`: module docs and the root modules row +- `../fonoteka.go/plugins/golem15/fonoteka/models/album_search.go`: Album document, schema and medium map +- `../fonoteka.go/plugins/golem15/fonoteka/search.go`: `settingsGate`, `wireSearch`, the typesense import +- `../fonoteka.go/plugins/golem15/fonoteka/plugin.go`: Boot calls `wireSearch` +- `../fonoteka.go/config/search.yaml`, `../fonoteka.go/README.md`: config and env mapping +- `../fonoteka.go/plugins/golem15/fonoteka/search_smoke_test.go`: the fake Typesense and both smoke tests + +## Decisions Made + +See `key-decisions` in the frontmatter. The most consequential are the two GORM findings (callback ordering and statement cloning) that made single-statement and plain-transaction writes silently skip sync in the Task 1 code. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Sync callbacks ran after GORM's own commit** +- **Found during:** Task 2 (RED run) +- **Issue:** `After("gorm:after_create")` alone makes GORM append the callback to the end of the chain, past `gorm:commit_or_rollback_transaction` and `lagoon:after_commit`. On a single-statement write, `lagoon.AfterCommit` buffered the work after the flush had already run, so nothing synced. The Task 1 smoke test only used `lagoon.Transaction` and missed it. +- **Fix:** each callback also declares `Before("gorm:commit_or_rollback_transaction")`. +- **Files modified:** `modules/beachcomber/beachcomber.go` +- **Verification:** subtest "a plain create syncs after its implicit commit" +- **Committed in:** `9543e65` + +**2. [Rule 1 - Bug] The gate read the settings row through the Album statement** +- **Found during:** Task 2 (RED run, debug prints) +- **Issue:** on a callback's handle, `db.Session(&gorm.Session{NewDB: true, Context: ctx})` clones the write's statement. The gate's `WithContext` continued from that clone, so it queried with the Album model and table and read an album row (`id 2`, `false`), which counted as off. Plain-transaction and single-statement writes were skipped silently. +- **Fix:** `cleanSession` (`Session`, then `Clauses()`, then `Session{NewDB}`) gives the gate and the document builder an empty statement on the same connection. +- **Files modified:** `modules/beachcomber/sync.go`, `modules/beachcomber/searchable.go` (Gate doc) +- **Verification:** subtests for the plain transaction, the implicit commit, soft delete and zero collection_id +- **Committed in:** `9543e65` + +**3. [Rule 2 - Missing critical] Savepoint around sync reads inside a caller's plain transaction** +- **Found during:** Task 1 +- **Issue:** a failed gate or reload read (for example no settings table yet) inside a plain `gorm` transaction would abort the caller's Postgres transaction, and the write would fail because of search. +- **Fix:** `inSavepoint` wraps the reads and rolls back to the savepoint on error, the same as lighthouse. +- **Files modified:** `modules/beachcomber/sync.go` +- **Committed in:** `3e1f3e6` + +--- + +**Total deviations:** 3 auto-fixed (2 bugs, 1 missing critical). +**Impact on plan:** all three were needed for the D-20 behaviour the plan requires. No scope creep. Two related latent issues in 11-01 and 11-03 code are logged in `deferred-items.md` and were not fixed: +- lighthouse broadcast callbacks have the same After-only ordering; +- lagoon's after-commit handle carries the write's statement. + +## Issues Encountered + +- `gsd-tools check tdd-red-evidence` reads surefire XML with the regex `name="…"`, which first matches inside `classname="…"`. A `` must list `name` before `classname`, or the target is never matched. This was worked around in the evidence record; it is not a project issue. +- A `cp` aliased to `cp -i` in the shell hung one debug command; `/bin/cp -f` was used afterwards. No repository impact. + +## Known Stubs + +None. `Album.ShouldBeSearchable` returning `true` is intentional; the settings Gate is the kill-switch, as the plan's flagged assumption states. + +## Threat Flags + +None. The only new trust boundary is the outbound Typesense client, which is covered by T-11-07, T-11-25, T-11-26 and T-11-27. No inbound endpoint was added. + +## User Setup Required + +None for tests. To index in a deployment, set `SUMMER_SEARCH__TYPESENSE__API_KEY` (and the host and port if they differ from `localhost:8181`), then turn on `search_use_typesense` in the admin settings. + +## Next Phase Readiness + +- Phase 12 can query `svc.Engine().SearchIDs(ctx, svc.IndexName(&models.Album{}), beachcomber.Query{QueryBy: models.AlbumSearchQueryBy, FilterBy: "collection_id:=…"})`. It must re-gate the ids in SQL. +- Bulk and CSV paths (Phase 13) should call `Service.Sync` per row. Statements without a primary key are not synced. +- Plan 11-07 (unit tests) should pick up both `deferred-items.md` entries and add engine-level tests for `typesense.Engine` without Postgres. + +## Self-Check: PASSED + +- All 11 key files exist on disk. +- Commits found: `3e1f3e6` and `9543e65` in summercms.go; `c4a7449`, `3b32946` and `0ce04ab` in fonoteka.go. +- Task 1 and Task 2 acceptance criteria were re-run and all pass. +- Plan verification passed: + - summercms.go: `go vet ./... && go test ./...` is green. + - fonoteka.go: the full vet and test command is green. + - `TestAlbumSearchSmoke` and `TestAlbumSearchDeleteAndFailures` report `--- PASS` under `-race`, with no SKIP and no DATA RACE. + +--- +*Phase: 11-jobs-realtime-and-search-infrastructure* +*Completed: 2026-09-30* diff --git a/.planning/phases/11-jobs-realtime-and-search-infrastructure/deferred-items.md b/.planning/phases/11-jobs-realtime-and-search-infrastructure/deferred-items.md new file mode 100644 index 0000000..4ad9747 --- /dev/null +++ b/.planning/phases/11-jobs-realtime-and-search-infrastructure/deferred-items.md @@ -0,0 +1,11 @@ +# Phase 11 deferred items + +Out-of-scope findings logged by plan executors. Not fixed; each names the plan that owns the code. + +## From plan 11-05 (found while debugging beachcomber) + +1. **lighthouse broadcast callbacks run after GORM's own commit on single-statement writes (plan 11-03 code).** + `modules/lighthouse/broadcast.go` `installCallbacks` registers `lighthouse:after_create`, `lighthouse:after_update` and `lighthouse:after_delete` with only an `After("gorm:after_*")` anchor. GORM's callback sorter appends such a callback to the end of the already sorted chain, which is past `gorm:commit_or_rollback_transaction`. 11-05 observed this directly for the identical beachcomber registration: `InstanceGet("gorm:started_transaction")` was set, but `ConnPool` was already the `*sql.DB`. For a plain `gdb.Create` outside any explicit transaction, the broadcast job is therefore enqueued after the commit and outside the write's transaction, not "on the write's `*sql.Tx`" as the lighthouse README states. A failed write still broadcasts nothing, because `db.Error` is set. Writes inside `lagoon.Transaction` or a plain `gorm` transaction are unaffected. Fix: add `.Before("gorm:commit_or_rollback_transaction")` to the three registrations (beachcomber does this since 9543e65), and add a test for a single-statement write. Candidate owner: plan 11-07 (unit tests). + +2. **lagoon hands AfterCommit callbacks a handle that carries the write's statement (plan 11-01 code).** + `modules/lagoon/transaction.go` `flushStatementAfterCommit` builds its handle with `db.Session(&gorm.Session{NewDB: true, Context: ctx})`, and `AfterCommit`'s immediate path passes the callback's own `db`. Because a Context is set, `Session` clones the write's statement (model, table, clauses). A later `WithContext` on that handle continues from the clone, so a query then runs through the written model's statement. beachcomber defends itself with `cleanSession` (9543e65). Any other `AfterCommit` user that calls `WithContext` or `Session` without `NewDB` on the handle it receives would hit the same bug. Fix: pass `db.Session(&gorm.Session{NewDB: true, Context: ctx}).Clauses().Session(&gorm.Session{NewDB: true})`, or document the requirement. Candidate owner: plan 11-07.