troed/reflect
troed/reflect
Watch
1
0
Fork
You've already forked reflect
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat: shared matcher (phase 2A) — embeddings, /match, attribution #1

Merged
troed merged 33 commits from feat/shared-matcher into main 2026-10-15 12:11:50 +02:00
Conversation 0 Commits 33 Files changed 31 +6034 -37
troed commented 2026-10-15 12:10:52 +02:00
Owner
Copy link

Summary

Phase 2A of the shared-knowledge design — the hub-side per-prompt lesson matcher. This contains no TypeScript and no in-session injection; the OpenCode plugin is phase 2B.

  • lesson_vec + match_stat tables — additive (CREATE TABLE IF NOT EXISTS); existing rows are untouched.
  • reflect/embed.py — a pluggable embedder: local ONNX MiniLM by default (optional [embed] extra: onnxruntime + tokenizers, imported lazily), an HTTP /v1/embeddings override when configured, a deterministic FakeEmbedder for tests, and fetch_bundle for one-time model provisioning.
  • reflect/match.py — cosine ranking over normalised vectors with a similarity floor, per-session dedupe, a shared-block ranking lift, and a read-only FTS5 keyword fallback. Matching never writes to the store.
  • POST /match (read token) and POST /attribution (worker token) on the existing MCP Starlette apps, token-guarded by the existing bearer wrapper. /match is fail-open: any internal error is a benign empty result, never a 5xx.
  • reflect similar and reflect embed fetch CLI commands.
  • Matcher stats — pointers injected, injection→pull conversion, match calls, floor rejects.
  • Nightly vector refresh — hub/local only; failure never changes a run's status.

Design

  • Spec: docs/superpowers/specs/2026-10-14-shared-memory-design.md (§6.1, §6.2, §7, §10, §14, §15, §16)
  • Plan: docs/superpowers/plans/2026-10-15-shared-matcher.md

Tests

489 passed, 1 skipped (uv run pytest). The skip is the env-gated real-ONNX integration test, which passes when a model bundle is provisioned. The ONNX inference contract (padding, input/output names, normalisation) is additionally pinned without onnxruntime via an injected fake session.

Existing strength arithmetic and every pre-existing reflect stats figure are unchanged — matching and attribution are evidence only.

Out of scope

The OpenCode plugin, pointer injection, in-session dedupe/cache, /attribution reporting from a session, and the host-side e2e (all phase 2B, which waits on a runtime probe of the plugin injection point).

Deferred

  • Hub-side verification against a copy of the live lessons.db (the plan's Task 8 Step 5) — not runnable off the hub.
  • An index and a retention policy for match_stat.
## Summary Phase 2A of the shared-knowledge design — the **hub-side per-prompt lesson matcher**. This contains no TypeScript and no in-session injection; the OpenCode plugin is phase 2B. - **`lesson_vec` + `match_stat` tables** — additive (`CREATE TABLE IF NOT EXISTS`); existing rows are untouched. - **`reflect/embed.py`** — a pluggable embedder: local ONNX MiniLM by default (optional `[embed]` extra: `onnxruntime` + `tokenizers`, imported lazily), an HTTP `/v1/embeddings` override when configured, a deterministic `FakeEmbedder` for tests, and `fetch_bundle` for one-time model provisioning. - **`reflect/match.py`** — cosine ranking over normalised vectors with a similarity floor, per-session dedupe, a shared-block ranking lift, and a **read-only** FTS5 keyword fallback. Matching never writes to the store. - **`POST /match`** (read token) and **`POST /attribution`** (worker token) on the existing MCP Starlette apps, token-guarded by the existing bearer wrapper. `/match` is fail-open: any internal error is a benign empty result, never a 5xx. - **`reflect similar`** and **`reflect embed fetch`** CLI commands. - **Matcher stats** — pointers injected, injection→pull conversion, match calls, floor rejects. - **Nightly vector refresh** — hub/local only; failure never changes a run's status. ## Design - Spec: `docs/superpowers/specs/2026-10-14-shared-memory-design.md` (§6.1, §6.2, §7, §10, §14, §15, §16) - Plan: `docs/superpowers/plans/2026-10-15-shared-matcher.md` ## Tests 489 passed, 1 skipped (`uv run pytest`). The skip is the env-gated real-ONNX integration test, which passes when a model bundle is provisioned. The ONNX inference contract (padding, input/output names, normalisation) is additionally pinned without `onnxruntime` via an injected fake session. Existing strength arithmetic and every pre-existing `reflect stats` figure are unchanged — matching and attribution are evidence only. ## Out of scope The OpenCode plugin, pointer injection, in-session dedupe/cache, `/attribution` reporting from a session, and the host-side e2e (all phase 2B, which waits on a runtime probe of the plugin injection point). ## Deferred - Hub-side verification against a copy of the live `lessons.db` (the plan's Task 8 Step 5) — not runnable off the hub. - An index and a retention policy for `match_stat`. 
- Task 2 breadth counts action IN ('useful','judge_useful'), not judge_useful alone
- Task 5 action set is judge_useful / judge_not_useful / retrieve_observed / inject,
  with retrieve_observed deliberately not spelled retrieve (stats counts every retrieve row)
- Task 7 emits retrieve_observed for a get_lesson part and nothing for search_lessons
- Task 7 no longer touches reflect/opencode_db.py
Review found three defects in the Task 7 miner:
- a rate_lessons entry keyed lesson_id (the shape an agent produces when it
  lifts the key from get_lesson) was silently dropped, though the tool documents
  the alias and Store.rate_lessons accepts it;
- a JSON verdict that is a list or object raised TypeError out of
  scan_session_usage, aborting collect_usage and losing every session's records
  for the night;
- the one success-path wiring test never ran the success branch, because the
  fake hub's record_usage raised AttributeError that the try/except under test
  swallowed, leaving the print and the returned counts uncovered.
tests/test_db.py's exact-set assertion on CATEGORIES is updated in step, since it cannot pass with the fifth category.
C1: pins were gated by the eligibility rule they exist to bypass.
shared_candidates had already dropped a pinned lesson with no breadth or a
decayed strength before select_members' pin loop could see it. Add
Store.pinned_candidates() (status + exclusion only) and feed selection a
second source; shared_candidates' own contract is unchanged.

C2: attribution was collected only in client mode, so on a single machine
(the default local/hub deployment) no judge_useful row was ever written and
breadth stayed empty. run_nightly's local/hub branch now runs the same
collection, guarded and wrapped so it cannot change the run's status.

I1: clients mirrored SHARED.md but never installed the pointer, leaving a
file no agent is told to read. _run_nightly_client now calls ensure_pointer
next to the mirror, inside the same guard and try/except.

I2: reflect shared now warns when a pin does not render (over the pin cap,
or failing the status/exclusion gate) in both show and pin.

I3: worker.md folds the convention instruction into the category bullet so
a model cannot send it as a substitute field and lose the lesson.

I4: record_usage type-checks project/session_id before the idempotency
probe, so a non-string wire value is skipped instead of raising mid-batch
and leaving a half-applied INSERT pending.

M4: run_shared_pass reports zero promoted/evicted on a write error, since
the block on disk did not change.

Also: pin shared_pointer_path under tmp_path in tests/test_shared.py, and
correct the spec's breadth formula and the breadth_by_lesson rationale.
A constructed HttpEmbedder is never probed, so an unreachable embed_url (or
one answering 501) only fails inside embed(); the same applies to an ONNX
inference failure. Absorb EmbedderUnavailable in match_query and fall through
to the shared keyword-fallback block, per spec section 10.

Also make the FTS-fallback tests hermetic: they relied on the ambient
build_embedder(cfg) returning None, which breaks on a host with a bundle.
Two carried review findings and Important 2 from the final whole-branch
review of the shared matcher.

- score_lessons skipped nothing: a lesson_vec row whose blob is not
  4 * dim bytes (a truncated or foreign write) reached struct.unpack and
  raised. `reflect similar` has no route-level except, so one bad row
  crashed the scan; such a row is now skipped. refresh_vectors gets the
  same guard on the vector an embedder hands back, where struct.pack
  would have raised instead.
- match_query gains `max_pointers`, overriding cfg.match_max_pointers on
  both ranking routes (the embedding cap and the FTS candidate pool
  size). `reflect similar --limit` passes it through: the CLI used to
  re-slice pointers that `rank` had already capped, so `--limit 10`
  under a cap of 3 printed 3 and could never widen the ranking the
  threshold is tuned with. Omitted, the configured cap still applies.
- `match_query`'s `embedder` default is now a sentinel: an omitted
  argument builds one, an explicit None means "no backend". The /match
  route needs to report a cached None without re-building (next commit).
- The client-mode gate message names the command the user ran
  (`reflect similar`) instead of a hard-coded list that omitted it.

Tests: struct-guard unit tests for both functions, --limit expansion at
the CLI and at both match_query routes, and the gate message.
Important 3 and Important 4 (plus the /match half of Important 1) from
the final whole-branch review of the shared matcher.

Lazy embedder: create_server built the embedder when the server was
constructed, so every per-session stdio `reflect mcp` server and the
worker app paid the local ONNX bundle's ~420 ms / ~200 MB even though
neither calls /match in this phase. It is now built on the first /match
request and cached, at most once per process; a sentinel separates "not
yet built" from a cached None ("no backend"), so a failed build is not
retried per request. An injected `embedder=` is still used as-is and
never rebuilt.

The route tests therefore no longer depend on the host: the tests that
build a client without an injected embedder (auth, missing query,
malformed body) pass a stub, so a hub with the bundle provisioned at the
default embed_model_dir can no longer turn them into real ONNX runs.

Attribution kind alias: spec §8 has the plugin report
{session_id, project, lesson_id, kind} with kind in {inject, retrieve,
judge_useful, judge_not_useful}, but Store.record_usage reads `action`
and knows `retrieve_observed`, not `retrieve`. A plugin written from §8
got 200 {recorded: 0, skipped: 1} — a silent no-op, and the
injection->pull figure read zero. The handler now maps `kind` onto
`action` before delegating (`retrieve` -> `retrieve_observed`, the other
three names unchanged). An explicit `action` still wins, and non-dict
records pass through so record_usage keeps owning validation and the
skip counts.

Tests: construction loads nothing, two /match calls build exactly once,
a build that answers None is cached rather than repeated, and the kind
wire shape records the alias plus the pass-through names.
Two carried review findings from the final whole-branch review.

HttpEmbedder.embed caught only OSError, so a non-OSError protocol or URL
error escaped as a crash instead of degrading to keyword search:
urlopen raises ValueError for an unknown scheme and an HTTPException for
a bad status line or a premature close (URLError is an OSError subclass,
now named alongside them). match_query only degrades on
EmbedderUnavailable, so these are mapped to it.

The ONNX contract is now pinned without onnxruntime: a fake session and
fake tokenizer are injected into OnnxEmbedder, asserting right-padding to
the batch max with a zeroed mask, token_type_ids fed only when the
session declares that input, the sentence_embedding output consumed, and
L2-normalised rows. The provisioned-bundle test stays as the integration
check.

test_build_embedder_prefers_url_then_onnx_then_none also stops
depending on the host: it asserted None against Config()'s default
embed_model_dir, which is exactly where `reflect embed fetch` provisions
the bundle, so provisioning the hub turned the assertion red. It now
points the default-shaped config at an empty directory.
Two carried review findings from the final whole-branch review.

`shared pointers injected` and `inject->pull conversions` were 24-char
labels in a 21-char label field, so their values sat four columns right
of the rest of the ACTIVITY block; they are now `shared pointers` and
`inject->pull pairs`, aligned with the lines around them. A test pins
both lines to the block width.

upsert_vector's docstring claimed a stale old-model vector is
"invisible rather than deleted", but lesson_id is the primary key, so a
re-embed replaces that row whatever model wrote it. Reworded to say so,
and to keep the invisibility claim for a vector another embedder wrote
for a lesson this one has not touched (which is load_index's filter).
The plan's Task 5 interfaces: the two endpoints' request and response
JSON, their transports and tokens (read server / worker server), and
their failure behaviour (a blank query is 400; anything else answers 200
with zeros or no pointers rather than 5xx). Also notes that `--limit`
may exceed the configured cap now that it widens the ranking.
troed merged commit 1dd724fce4 into main 2026-10-15 12:11:50 +02:00
troed deleted branch feat/shared-matcher 2026-10-15 12:11:51 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
troed/reflect!1
No description provided.