8 Commits

Include renames

Author	SHA1	Message	Date
bvandeusenandClaude Opus 5	8be555d6dd	feat(telemetry): tell a ranker decline from a repeat before the observation window opens (#3497) ... Making the rule arms log every call exposed a second ambiguity in the same row. `result_count == 0` is two unrelated events wearing one number: - the ranker found nothing above the bar — the only evidence a threshold is set too high; and - the ranker found only what this session had already been shown — which says nothing whatever about the bar. A long session excludes its way into the second, so the arm reads worse the longer it runs correctly. Rows written now carry the ambiguity permanently, which is why this lands before any watch period rather than after. `retrieval_logs.suppressed_count` (0095, nullable) holds what the caller dropped as already-shown. Both rule arms report it; they filter in Python and always know. The note arms pass exclusions INTO semantic_search_notes and never see what was dropped, so they store NULL. THE NULL IS LOAD-BEARING. It means "not measured here", and the readout renders it as `suppression: null` rather than a zeroed dict. Defaulting to 0 would let an unmeasured surface read as a perfectly clean one — the same substitution of an artifact for a measurement that #3311 made. No backfill, for the same reason: existing rows genuinely do not know. `retrieval_telemetry`'s `sources` gains `suppression` with `measured_calls`, `calls_with_suppression` and `zero_because_already_shown`; subtract the last from `zero_result_calls` for the true ranker declines. The MCP tool docstring says to read the two together and warns against reading the null as a zero. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ	2026-09-03 21:12:38 -04:00
bvandeusenandClaude Opus 5	8b9b3a1d9b	feat(telemetry): the preload emits, and the always-on set stops being unfalsifiable (#3473) ... The ranked rule arm became measurable in M333. The preload did not — and that is the surface whose value is actually in question. `list_always_on_rules`, the SessionStart block and every `rules_payload` caller handed rules over wholesale and emitted nothing, so the resident set's token cost was certain and its usefulness could not be tested even in principle. Bulk deliveries now record as AMBIENT, beside the ranked count and never inside pull-through. Folding them in would mean growing the always-on set depressed the arm's measured precision and trimming it flattered the arm, neither for any reason to do with the arm. `RANKED_SOURCES` inverts the note twin's `AMBIENT_SOURCES` deliberately: there is one ranked rule source and this change adds seven bulk ones, so naming the rare half makes a forgotten surface default to ambient — under-counting it — rather than padding the denominator with surfacings nobody chose. Two lookalike call sites are deliberately left silent, with a test to keep them that way: the write-path etag arm and `rules_etag_for` read the rules to build or compare a MARKER and show nobody anything. No migration — `event` and `source` are plain Text with no CHECK (rule 36 does not apply). Snippet #2858 updated to the new `rules_payload` contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ	2026-09-02 23:06:40 -04:00
bvandeusenandClaude Opus 5	8901c904a9	feat(telemetry): retrieval_telemetry reports rule pull-through where it reported nothing (#3317) ... Milestone 333 step 3, the read half. Steps 1 and 2 built the table and filled it; until now nothing read it, and `usage` — sourced entirely from note_usage_events — described notes only while `sources` happily listed a write_path_rule row above it. A reader takes the aggregate as covering everything named above it. It did not. A SEPARATE `rule_usage` BLOCK, not folded into `usage`. Two reasons, and the second is the one that bites: the corpora differ by orders of magnitude, so a blended ratio would be the note ratio with noise on it and the rule arm would stay invisible inside it; and `usage` is what existing callers already read and compare across windows, so silently changing what it counts would move a number nobody was told had changed meaning. There is a test asserting rule events stay out of the note block. No `ambient` key, unlike the twin. Nothing surfaces a rule un-ranked — list_always_on_rules and enter_project hand rules over wholesale but emit no event — so there is no ambient class to subtract. The absence is a fact about the data, not an oversight, and it returns when a bulk loader starts emitting. Guarded separately, like `by_source`. This table did not exist a commit ago, and an instance running upgraded code against un-migrated schema would otherwise take down two readouts that work perfectly in order to report a third that cannot. On failure the FLAG is added and the SHAPE is kept — a caller must not have to choose between crashing on a missing key and quietly rendering zeros it has no right to. `pull_through` is None rather than 0.0 on an empty window, matching the note block. A ratio of zero asserts "rules were shown and none opened"; with an empty numerator and denominator that is a claim the data does not support, and it is the reading that would make a brand-new install look like a broken one. Also fixed, from #3311: the rule arm never timed its search, so it was the one source in the readout reporting a null p90_duration_ms — a gap that reads as "this surface is somehow not measurable" rather than "nobody passed the number". Both docstrings updated in the same change. The tool's is the agent-facing contract (rule 119) and it explicitly said rule surfacings were absent and had "no usage counter at all". Leaving that would have had a reader conclude the arm has zero pull-through rather than a separate one. Tests are integration for the reason the block above them is: real GROUP BYs and count(distinct) against a table a commit old, in a module whose one production outage was a SQL shape the database rejected inside a broad except. A mock would agree with whatever the code does, including nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcCs1CcQ1ormdnzSshKqvN	2026-09-02 17:25:11 -04:00
bvandeusen	0d4b155699	feat(telemetry): pull-through per surface, not just per corpus (#3311) ... The readout already grouped usage by source — `group_by(event, source)` — and the loop directly below it threw the source away, collapsing every surface into one corpus-wide ratio. So the question a threshold is actually tuned against, "is THIS surface worth its noise", could not be asked of any surface, while the data to answer it sat in the table. `usage.by_source` reports notes_surfaced / notes_pulled / pull_through per surface. The grain is the note, not the call: a pull records the door it came through, not the surface that led there, so grouping the pulled rows by source would answer a different question. Joining surfaced rows to pulled rows on note_id answers this one without the session identity #2085 declined to invent — at the cost of being an upper bound per surface, which the docstring says where it is read. Ambient surfaces report counts and a null ratio: nothing chose those records, so "surfaced often, opened never" is not a judgment about them. A surface that genuinely produced nothing reports 0.0, which must not look like the null. The join is guarded separately from the two reads above it. #2663 was a novel SQL shape the database rejected inside a broad except; this is the novel shape here, and it must not take down two readouts that work. Tests are integration for that same reason — a mock passes on a query Postgres refuses. They pin the distinct-first property (three surfacings of one note are one note), the ambient null, and the LIKE escape, since an unescaped `mcp_%` also matches `mcpXget_note` and nothing else in the payload would show the difference.	2026-08-31 15:52:17 -04:00
bvandeusenandClaude Opus 5	02c1e37620	feat(rules): the write path can notice a standing rule it was never given (#3031, milestone 307 step 5, hook arm) ... A conditional rule is not resident, so a session can be about to violate one it was never handed. This arm notices: when what is being written resembles a rule's trigger, the hint names it and says to read it before deciding it does not apply. A SUGGESTION, and the plan was wrong about why it could be more. It claimed the hook "already resolves a path to an area" — it does not, and nothing in Scribe maps a path to a System or a canonical area (build_write_path_hint resolves paths against snippet LOCATIONS, a different index; the learned-alias idea belongs to another project). Correction logged on the task. Rather than invent path→area inference to make a stale claim true, the arm does what D7 already decided and what this surface already IS: tags bind at enter_project, meaning suggests here. The header of the hook says NEVER BLOCKS; dressing a hint up as binding would have been the actual mistake. CONDITIONAL RULES ONLY. An always-on rule is already in the session, so re-offering it is noise — and noise on a hint that fires on every write is how a hint gets ignored. Telemetry goes to retrieval_logs, NOT note_usage_events, and that is a correctness call rather than a preference: note_usage ids are REMAPPED on a backup restore, so a rule id written there would come back attached to whatever note took that number — silently corrupting the evidence the next true-up is supposed to read. retrieval_logs is never restored and `source` already separates surfaces. record_retrieval's `results` type widened to match what it actually needs (an `.id`), instead of passing a Rule to something annotated Note. The rule dedup gets its OWN state file and query parameter, like the three channels before it — #2708's lesson was that one shared channel lets a hint of one class silence a different class that had never been shown. Plugin version bumped: a hook change clients cannot see did not ship (#1040). The stub is autouse in conftest rather than added to forty-odd call sites: the arm loads an embedding model, and every existing test that stubs the NOTES search would otherwise pull a real model in through the one arm it had no way to know about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-26 16:35:36 -04:00
bvandeusenandClaude Fable 5	64bfa5725f	feat(telemetry): a read surface over retrieval_logs — the tuning loop had no read half (#2975) ... `retrieval_logs` was write-only. `record_retrieval` inserted rows and nothing in the tree ever selected from them: the only `select()` over RetrievalLog lived in a test. So #1038's gate — "build the reranker once telemetry shows precision is the bottleneck" — was unsatisfiable by construction, and the one real tuning decision on record (the 0.68 write-path threshold, #2223) had to be reached by hand-probing the live instance with eight payloads. This adds the half that was missing. `retrieval_summary(user_id, days=30)` returns two aggregates side by side, each read from the table built for it — NOT a join. NoteUsageEvent's docstring is explicit that the two are complements ("RetrievalLog tunes the threshold, this tunes the corpus") and that RetrievalLog's JSONB `result_ids` cannot be indexed at the per-note grain, so correlating through it would be both slower and less honest than reading each source directly. That corrects the approach sketched on the task. - `sources`, per surface: calls, zero_result_calls, cleared_threshold (how often the best hit beat the threshold in force for THAT call), the top_score spread as p10/p50/p90/min/max, avg_result_count, p90 duration. Zero-result calls are counted apart from low-scoring ones — they are a different failure and averaging them together would hide both. - `usage`, from note_usage_events: ranked surfacings, ambient surfacings, and pulls split into `pulled_by_agent` / `pulled_by_human`. That split is not decoration. NoteUsageEvent's own comment says the mcp_/rest_ prefix is load-bearing and names #1038 while saying so: "is this dead weight?" is answered by any pull, "was that injected line useful?" only by an agent pull. `pull_through` exists to answer the second, so it counts agent pulls over ranked surfacings; both halves ship so the first stays answerable. Two things the code made me get right rather than guess: - Distinct-note counts get their own queries. `count(distinct note_id)` per (event, source) group cannot be summed across groups — a note surfaced by two sources is one distinct note and would be counted twice. A wrong number labelled "distinct" is worse than no number. - No CASE in the GROUP BY. #2663 is the bug where a second case() rendered its own expanding bind names, Postgres rejected the query, a broad except swallowed it, and every counter read zero in production while mocked tests passed. Grouping on raw `source` and classifying in Python cannot fail that way. For the same reason the readout distinguishes `read_failed` from an empty window, and its tests are integration against real Postgres — percentile_cont ... WITHIN GROUP only proves it parses against a database. Exposed as the `retrieval_telemetry` MCP tool, added to `_READ_ONLY_TOOLS`: it mutates nothing, but its name carries no read prefix, so the completeness test cannot derive it and it would otherwise have failed closed for read-only keys in silence — the same reason `enter_project` is spelled out there. Docs updated to name both exceptions rather than leave the rule looking derivable. Scoped to the caller's own telemetry: a retrieval log records what one user's agent asked for, query text included, and is not a shared record kind — the owner filter is the whole access rule, not a shortcut past access.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-08-23 22:48:54 -04:00
bvandeusenandClaude Fable 5	77acee9239	fix(telemetry): usage counters get canaries, task references, and the missing integration tests (#2663) ... The deployed instance ran with every usage counter at zero while surfacing demonstrably fired. Every unit test was green because every unit test mocked either _schedule or the session — the two functions that touch the database ran against real Postgres nowhere. Both telemetry writers also held no reference to their fire-and-forget tasks (the loop keeps only weak ones), and swallowed every failure into logger.debug, so a total outage was indistinguishable from an unused corpus. - note_usage + retrieval_telemetry keep strong task references until done - failures log at WARNING; note_usage additionally drops one AppLog error row per process per site, so the admin UI shows the outage without host access - integration tests cover _insert_events -> usage_for_notes and the full record_pulled chain on a running loop, splitting the write and read halves so a failure names its side Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-08-14 21:45:32 -04:00
bvandeusenandClaude Opus 4.8	807f478cac	feat(search): retrieval telemetry — log every semantic retrieval ... Add retrieval_logs (migration 0068) + services/retrieval_telemetry with a fire-and-forget record_retrieval(), wired into the MCP search tool (source=mcp_search) and the REST search route (source=rest_search). Captures query, effective params, and the per-result score distribution so KB-injection thresholds can be tuned from data rather than guessed. Scribe: project 2, milestone 93, task 1032. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xz4j1H7pjYSjKsEpgcNH5E	2026-06-22 20:10:15 -04:00