Merge pull request 'The wide net becomes a pull, and a tuned number carries the space it was measured in' (#164) from dev into main
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m41s
CI & Build / Build & push image (push) Successful in 19s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m41s
CI & Build / Build & push image (push) Successful in 19s
This commit was merged in pull request #164.
This commit is contained in:
@@ -0,0 +1,51 @@
|
||||
"""retrieval_tuning_events carries the space each number was measured in (#4104)
|
||||
|
||||
Revision ID: 0104
|
||||
Revises: 0103
|
||||
Create Date: 2026-09-17
|
||||
|
||||
Milestone 416 step 6. A retrieval floor is a cosine distance in ONE embedding
|
||||
model's geometry, computed over documents cut one particular way. Change the
|
||||
model and every score moves at once; change the chunker and the same record
|
||||
embeds different text. Either way a number chosen before the change is a
|
||||
measurement of something that no longer exists — and today nothing records
|
||||
which world it was chosen in, so the staleness is unknowable rather than
|
||||
merely unknown.
|
||||
|
||||
`CHUNKER_VERSION` already solved exactly this for documents: stored per row, so
|
||||
the startup backfill re-embeds precisely what is stale instead of wiping the
|
||||
table. These two columns are that idea applied to the tuned numbers.
|
||||
|
||||
TWO COLUMNS, NOT ONE (rule 149). A reader has to be able to say WHICH half
|
||||
moved: a new embedding model and a re-cut document shape invalidate the same
|
||||
numbers for different reasons, and a fused `"<model>@<n>"` could only report
|
||||
that something changed.
|
||||
|
||||
NULLABLE, and not backfilled. The rows already in this table were written
|
||||
under something, but naming it would be inventing a fact — the honest value is
|
||||
"unstamped", which is a different answer from a model name that might be wrong.
|
||||
`current_settings` reports an unstamped dial as exactly that.
|
||||
"""
|
||||
import sqlalchemy as sa
|
||||
from alembic import op
|
||||
|
||||
revision = "0104"
|
||||
down_revision = "0103"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.add_column(
|
||||
"retrieval_tuning_events",
|
||||
sa.Column("embedding_model", sa.Text(), nullable=True),
|
||||
)
|
||||
op.add_column(
|
||||
"retrieval_tuning_events",
|
||||
sa.Column("shape_version", sa.Integer(), nullable=True),
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_column("retrieval_tuning_events", "shape_version")
|
||||
op.drop_column("retrieval_tuning_events", "embedding_model")
|
||||
@@ -178,6 +178,64 @@ interface TuningEvent {
|
||||
reason: string;
|
||||
}
|
||||
|
||||
// What a dial's number was measured against, and whether that space has moved
|
||||
// (#4104). Mirrors the `calibration` block `current_settings` returns per dial.
|
||||
// `stale` is nullable on purpose: a dial moved before Scribe recorded stamps is
|
||||
// UNKNOWN rather than fine, and rendering the two the same would hide the case
|
||||
// most likely to be wrong.
|
||||
interface DialCalibration {
|
||||
source: string;
|
||||
embedding_model: string | null;
|
||||
shape_version: number | null;
|
||||
model_changed: boolean | null;
|
||||
shape_changed: boolean | null;
|
||||
stale: boolean | null;
|
||||
}
|
||||
|
||||
interface SurfaceRow {
|
||||
surface: string;
|
||||
calibration: Record<string, DialCalibration>;
|
||||
}
|
||||
|
||||
const surfaceRows = ref<SurfaceRow[]>([]);
|
||||
|
||||
// Flattened to one line per dial that needs looking at, because the operator's
|
||||
// question is "which numbers do I no longer trust", not "tell me about each of
|
||||
// six surfaces". Anything calibrated against the live model contributes
|
||||
// nothing — the panel below renders only when this is non-empty, so a healthy
|
||||
// install never sees it. A warning that is always on the screen is one nobody
|
||||
// reads when it finally means something.
|
||||
const staleDials = computed(() =>
|
||||
surfaceRows.value.flatMap((row) =>
|
||||
Object.entries(row.calibration ?? {})
|
||||
.filter(([, cal]) => cal.stale !== false)
|
||||
.map(([dial, cal]) => ({
|
||||
key: `${row.surface}.${dial}`,
|
||||
surface: row.surface,
|
||||
dial,
|
||||
why:
|
||||
cal.source === "unstamped"
|
||||
? "changed before Scribe recorded what it was measured against"
|
||||
: cal.model_changed && cal.shape_changed
|
||||
? `measured under ${cal.embedding_model} at document shape ${cal.shape_version} — both have changed since`
|
||||
: cal.model_changed
|
||||
? `measured under ${cal.embedding_model}, which is no longer the embedding model`
|
||||
: `measured at document shape ${cal.shape_version}, and documents are cut differently now`,
|
||||
})),
|
||||
),
|
||||
);
|
||||
|
||||
async function loadSurfaces() {
|
||||
try {
|
||||
const res = await apiGet<{ surfaces: SurfaceRow[] }>("/api/retrieval/surfaces");
|
||||
surfaceRows.value = res.surfaces ?? [];
|
||||
} catch {
|
||||
// Same reasoning as the history below: unreadable calibration is not worth
|
||||
// a toast on page load, and the dials above still work.
|
||||
surfaceRows.value = [];
|
||||
}
|
||||
}
|
||||
|
||||
async function loadTuningHistory() {
|
||||
loadingTuning.value = true;
|
||||
try {
|
||||
@@ -292,6 +350,7 @@ async function saveKbInject() {
|
||||
// otherwise the panel shows a trail that is stale by exactly the change
|
||||
// the operator is looking at it to confirm.
|
||||
await loadTuningHistory();
|
||||
await loadSurfaces();
|
||||
} catch {
|
||||
toastStore.show('Failed to save auto-inject settings', 'error');
|
||||
} finally {
|
||||
@@ -767,6 +826,7 @@ onMounted(async () => {
|
||||
kbReportPrefTopK.value = allSettings.kb_reportpref_top_k;
|
||||
}
|
||||
await loadTuningHistory();
|
||||
await loadSurfaces();
|
||||
if (allSettings.kb_duplicate_threshold_snippet !== undefined) {
|
||||
kbDupThresholdSnippet.value = allSettings.kb_duplicate_threshold_snippet;
|
||||
}
|
||||
@@ -1772,6 +1832,32 @@ async function deleteUser(userId: number) {
|
||||
moves the dial with the argument attached. This panel is the other
|
||||
half of that bargain — a change made on your behalf is one you can
|
||||
read, disagree with, and set back by hand above. -->
|
||||
<!-- STALENESS (#4104). A bar is a cosine similarity, which only means
|
||||
something inside one embedding model's geometry over documents cut
|
||||
one particular way. Change either and every number above keeps
|
||||
applying while describing nothing. Shown ONLY when something has
|
||||
actually moved, so that seeing it at all is the signal. -->
|
||||
<div v-if="staleDials.length" class="calibration-warning">
|
||||
<h4 class="calibration-warning-title">
|
||||
{{ staleDials.length }}
|
||||
{{ staleDials.length === 1 ? 'value was' : 'values were' }}
|
||||
measured against something that has changed
|
||||
</h4>
|
||||
<p class="field-hint">
|
||||
These bars are similarity scores, and a similarity score only means
|
||||
something inside the model that produced it. They still apply — they
|
||||
just no longer measure what they were set to measure. Nothing is
|
||||
adjusted automatically; ask Claude to migrate them and it will carry
|
||||
each one across at the same selectivity, then check the result.
|
||||
</p>
|
||||
<ul class="calibration-list">
|
||||
<li v-for="d in staleDials" :key="d.key">
|
||||
<span class="tuning-surface">{{ d.surface }}</span>
|
||||
<span class="tuning-dial">{{ d.dial }}</span>
|
||||
<span class="calibration-why">{{ d.why }}</span>
|
||||
</li>
|
||||
</ul>
|
||||
</div>
|
||||
<div class="tuning-history">
|
||||
<h4 class="tuning-history-title">What has been tuned</h4>
|
||||
<p class="field-hint">
|
||||
@@ -4115,4 +4201,36 @@ async function deleteUser(userId: number) {
|
||||
font-size: 0.85rem;
|
||||
color: var(--fs-text-secondary);
|
||||
}
|
||||
/* Stale calibration (#4104). Tinted rather than loud: the numbers still work,
|
||||
they have merely stopped being measurements — that is a "come back to this",
|
||||
not an outage. It only renders when something is actually stale, which is
|
||||
what earns it the tint at all. */
|
||||
.calibration-warning {
|
||||
margin-top: var(--fs-space-5);
|
||||
padding: var(--fs-space-3);
|
||||
border: 1px solid var(--fs-warning);
|
||||
border-radius: var(--fs-radius-md);
|
||||
background: color-mix(in srgb, var(--fs-warning) 12%, var(--fs-surface-raised));
|
||||
}
|
||||
.calibration-warning-title {
|
||||
margin: 0 0 var(--fs-space-2);
|
||||
font-size: 0.95rem;
|
||||
color: var(--fs-warning-fg);
|
||||
}
|
||||
.calibration-list {
|
||||
list-style: none;
|
||||
margin: var(--fs-space-2) 0 0;
|
||||
padding: 0;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: var(--fs-space-1);
|
||||
font-size: 0.85rem;
|
||||
}
|
||||
.calibration-list li {
|
||||
display: flex;
|
||||
align-items: baseline;
|
||||
flex-wrap: wrap;
|
||||
gap: var(--fs-space-2);
|
||||
}
|
||||
.calibration-why { color: var(--fs-text-secondary); }
|
||||
</style>
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "scribe",
|
||||
"description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).",
|
||||
"version": "2026.09.17.0111",
|
||||
"version": "2026.09.17.1618",
|
||||
"author": {
|
||||
"name": "Bryan Van Deusen"
|
||||
},
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
name: using-scribe
|
||||
description: Use at the START of every session, and before answering anything about the operator's work or starting any task — establishes the Scribe-first reflex. You hold none of the operator's rules: they arrive by retrieval when your work matches one, and search(content_type="rule") is how you ask before a consequential act. Call enter_project when a repo/project is in scope. Then recall before acting, update over duplicate, plan in Scribe not in files.
|
||||
description: Use at the START of every session, and before answering anything about the operator's work or starting any task — establishes the Scribe-first reflex. You hold none of the operator's rules: they arrive by retrieval when your work matches one, and what_might_apply is how you ask before a consequential act — it returns the wide net of candidates with no bar. Call enter_project when a repo/project is in scope. Then recall before acting, update over duplicate, plan in Scribe not in files.
|
||||
---
|
||||
|
||||
# Using Scribe
|
||||
@@ -73,11 +73,21 @@ Two constraints on *how* that's achieved:
|
||||
|
||||
So "no rule arrived" means "nothing matched", never "no rule exists" — an
|
||||
empty session is not evidence of an empty rulebook. Retrieval fires when
|
||||
something asks: before a consequential act, `search(content_type="rule")` on
|
||||
what you are about to do, and pull a record's full statement with
|
||||
`get_rule(id)` when it is about to bite. When a project is in scope, pass
|
||||
its `project_id`: the answer is then the global rules plus that project's
|
||||
own, never another project's. `enter_project(id)` lists the project's own
|
||||
something asks, so ask — and reach for the tool that fits the moment:
|
||||
|
||||
- **Before a consequential act**, and before handing work back because you
|
||||
are unsure you may finish it: `what_might_apply("what you are about to
|
||||
do")`. It returns up to fifty ranked candidates with NO bar. The arms that
|
||||
push rules at you spend a budget of three and say nothing about what sat
|
||||
just underneath — right for something firing before every command, wrong
|
||||
for the one moment you actually want to be sure. Expect the tail to be
|
||||
noise; you are reading for the one record you would have missed.
|
||||
- **When you already suspect a particular rule**: reach for
|
||||
`search(content_type="rule")` to read full statements, or `get_rule(id)`
|
||||
when one is about to bite.
|
||||
|
||||
Either way, when a project is in scope pass its `project_id`: the answer is
|
||||
then the global rules plus that project's own, never another project's. `enter_project(id)` lists the project's own
|
||||
rules by title.
|
||||
|
||||
**`kind` says how much force a record carries, and it is never something to
|
||||
|
||||
@@ -44,8 +44,11 @@ client reads Agent Skills) and in each tool's description. The index:
|
||||
system. An `inception` key: ask what it inherits, then
|
||||
decide_project_inception.
|
||||
- RULES: nothing preloads; a rule arrives when your work matches it. Before a
|
||||
consequential act, search(content_type="rule"). Silence means
|
||||
nothing matched, not none. Rules bind; preferences guide.
|
||||
consequential act — or before handing work back unsure you may finish it —
|
||||
what_might_apply("what you are about to do"): the wide net, fifty ranked
|
||||
candidates, no bar. search(content_type="rule") reads one you already
|
||||
suspect. Silence means nothing matched, not none. Rules bind; preferences
|
||||
guide.
|
||||
- RECALL: search before acting, scoped with the active project_id.
|
||||
- RECORD: create_task; a fix is kind="issue". add_task_log as you go; status
|
||||
in_progress on start, done on finish. Tag system_ids as you write.
|
||||
@@ -137,6 +140,11 @@ _READ_ONLY_TOOLS = frozenset({
|
||||
# usual for these two: a session that cannot see the bar in force, or the
|
||||
# reason it was last moved, is a session that will move it again blind.
|
||||
"retrieval_surfaces", "retrieval_tuning_history",
|
||||
# The wide net (#4103) — ranked rule candidates with no bar, for the
|
||||
# moment before a consequential act. A pure read, and one a read key needs
|
||||
# most: it is the surface the "ask before acting" reflex calls, and a key
|
||||
# that could not reach it would be denied exactly the check it should run.
|
||||
"what_might_apply",
|
||||
})
|
||||
|
||||
# Every tool that WRITES, by name. Nothing reads this set at runtime — a tool
|
||||
@@ -175,6 +183,7 @@ _WRITE_TOOLS = frozenset({
|
||||
# retrieval tuning — a write in both senses: it moves the number the arm
|
||||
# reads, and it appends the reason to the audit trail (#4102).
|
||||
"tune_retrieval",
|
||||
"migrate_retrieval_floor",
|
||||
# trash
|
||||
"restore", "purge_trash",
|
||||
})
|
||||
|
||||
@@ -6,6 +6,7 @@ from `mcp.server.build_mcp_server`.
|
||||
"""
|
||||
from scribe.mcp.tools import (
|
||||
design_systems, milestones, notes, processes, projects, recent, repos, retrieval_tuning,
|
||||
wide_net,
|
||||
rulebooks, search, shapes, snippets, systems, tags, tasks, trash,
|
||||
)
|
||||
|
||||
@@ -14,6 +15,7 @@ def register_all(mcp) -> None:
|
||||
"""Register every tool module's tools on the given FastMCP instance."""
|
||||
search.register(mcp)
|
||||
retrieval_tuning.register(mcp)
|
||||
wide_net.register(mcp)
|
||||
notes.register(mcp)
|
||||
tasks.register(mcp)
|
||||
projects.register(mcp)
|
||||
|
||||
@@ -5,6 +5,7 @@ bar did; these say what the bar IS, and let it be changed with the argument
|
||||
attached.
|
||||
"""
|
||||
from scribe.mcp._context import current_user_id
|
||||
from scribe.services import retrieval_migration as migration_svc
|
||||
from scribe.services import retrieval_tuning as tuning_svc
|
||||
|
||||
|
||||
@@ -37,6 +38,33 @@ async def retrieval_surfaces() -> dict:
|
||||
are starting points: they were measured against one corpus with one
|
||||
embedding model and cannot be right for another install by construction.
|
||||
That is why this tool exists rather than a better set of defaults.
|
||||
|
||||
`calibration` says what SPACE each number was chosen in, per dial, and
|
||||
whether that space has moved. A floor is a cosine distance in one embedding
|
||||
model's geometry over documents cut one particular way; swap the model and
|
||||
every score shifts at once, re-cut the documents and the same record embeds
|
||||
different text. Either way the number is a measurement of something that no
|
||||
longer exists, and no amount of telemetry will say so — the scores simply
|
||||
come out different and the bar keeps applying.
|
||||
|
||||
Read the three fields apart:
|
||||
|
||||
- `stale: true` with `model_changed` — the geometry is new. Every floor on
|
||||
this install needs re-measuring, not adjusting; a number that meant "quite
|
||||
similar" in the old space means nothing particular in this one.
|
||||
- `stale: true` with `shape_changed` — the documents are cut differently.
|
||||
The scale still holds, but what a record embeds has changed, so which
|
||||
records clear a bar has.
|
||||
- `stale: null`, `source: "unstamped"` — the dial was moved before Scribe
|
||||
recorded this. It was measured under SOMETHING and there is no way to say
|
||||
what, which is a different answer from "it is fine". Treat it as worth
|
||||
re-measuring, and the next tuning call stamps it.
|
||||
|
||||
NOTHING IS RETUNED AUTOMATICALLY on the strength of this, here or anywhere.
|
||||
A stale stamp says a number is no longer a measurement; it does not say what
|
||||
the number should be. That judgement wants the same procedure as any other
|
||||
tuning change — `retrieval_telemetry(near_miss_samples=5)`, open the records
|
||||
it names, then `tune_retrieval` with what you read in the reason.
|
||||
"""
|
||||
return {"surfaces": await tuning_svc.current_settings(current_user_id())}
|
||||
|
||||
@@ -109,7 +137,53 @@ async def retrieval_tuning_history(surface: str = "", limit: int = 20) -> dict:
|
||||
}
|
||||
|
||||
|
||||
async def migrate_retrieval_floor(
|
||||
surface: str, sample: int = 200, apply: bool = False,
|
||||
) -> dict:
|
||||
"""Carry one surface's floor across an embedding-model or chunker change.
|
||||
|
||||
Reach for this when `retrieval_surfaces` reports a dial `stale` — and only
|
||||
then. A floor is a cosine similarity, so a new embedding model moves every
|
||||
score on the install at once and the stored number silently stops describing
|
||||
anything. Re-deriving six floors by reading telemetry is the work this
|
||||
avoids.
|
||||
|
||||
WHAT IT TRANSFERS. Not the number — the SELECTIVITY. A floor's real content
|
||||
is a decision about how much of what an arm sees is worth spending attention
|
||||
on, and that decision survives a change of units. This measures what fraction
|
||||
of the surface's recent calls the old floor admitted, re-scores those same
|
||||
queries under the current model, and proposes the value that admits the same
|
||||
fraction.
|
||||
|
||||
DRY RUN BY DEFAULT. With `apply=False` it returns the arithmetic and writes
|
||||
nothing. Read it before applying: the sample sizes, both score ranges, and
|
||||
whether the shift looks like a change of scale or like a corpus that has not
|
||||
finished re-embedding. A model change is the moment every number is
|
||||
uncertain at once, which is the worst moment to let a statistic move dials
|
||||
unattended.
|
||||
|
||||
AND IT IS STILL A STARTING POINT. Percentile-preserving means the new floor
|
||||
is as good as the old one was, not better — if the old floor was wrong, this
|
||||
faithfully carries the wrongness onto the new scale. It is the number to
|
||||
begin from while the surface collects enough calls to judge properly, which
|
||||
is `retrieval_telemetry(near_miss_samples=5)` and `tune_retrieval` as usual.
|
||||
|
||||
Args:
|
||||
surface: the surface name from `retrieval_surfaces`.
|
||||
sample: how many recent logged calls to re-score, default 200. Each one
|
||||
is an embedding plus a scan, so this is real work; a surface with
|
||||
only a handful of logged calls gives a percentile made of noise.
|
||||
apply: False (default) returns the proposal. True writes it, as an
|
||||
ordinary tuning event with the arithmetic in its reason — so a
|
||||
migrated floor is reviewable and revertible like any other.
|
||||
"""
|
||||
return await migration_svc.migrate_floor(
|
||||
current_user_id(), surface, sample=sample, apply=apply,
|
||||
)
|
||||
|
||||
|
||||
def register(mcp) -> None:
|
||||
mcp.tool(name="retrieval_surfaces")(retrieval_surfaces)
|
||||
mcp.tool(name="migrate_retrieval_floor")(migrate_retrieval_floor)
|
||||
mcp.tool(name="tune_retrieval")(tune_retrieval)
|
||||
mcp.tool(name="retrieval_tuning_history")(retrieval_tuning_history)
|
||||
|
||||
@@ -0,0 +1,218 @@
|
||||
"""The wide net — every candidate that might govern what you are about to do (#4103).
|
||||
|
||||
WHY THIS EXISTS
|
||||
|
||||
Milestone 416 step 5, from the operator's compromise:
|
||||
|
||||
"if you're worried about excluding potentially important data let limit it
|
||||
to 50 entries or something like that"
|
||||
|
||||
Fifty in the PUSH would be milestone 394 with extra steps — a wall the reader
|
||||
skims, with the governing record indistinguishable from forty-nine others.
|
||||
Fifty in a PULL is a different object: it arrives only when a session asks, it
|
||||
crowds nothing out, and it is the honest home for "do not exclude potentially
|
||||
important data".
|
||||
|
||||
WHY IT IS NOT `search(content_type="rule")`
|
||||
|
||||
That tool is the DEEP pull and should stay that way — `_search_rules` returns
|
||||
`statement`, `why` and `how_to_apply` in full, on the reasoning that a caller
|
||||
who went looking deserves the whole record rather than a summary to re-fetch.
|
||||
Six results already run to thousands of tokens.
|
||||
|
||||
This is the SHALLOW pull: many candidates, each just enough to decide whether
|
||||
to open it. Opposite trade-off, so it is a second tool rather than a bigger
|
||||
`limit` on the first.
|
||||
|
||||
WHY THE TAIL IS TRUNCATED RATHER THAN FULL
|
||||
|
||||
Because the step's premise needed correcting. The task said fifty "costs
|
||||
nothing", and `_rule_hint_line` had already measured otherwise: a line runs
|
||||
~143 tokens once its trigger is rendered, and #3855 tripled trigger lengths
|
||||
across the corpus. Fifty of those is ~7,000 tokens — cheap next to an arm that
|
||||
fires before every Bash call, but not free, and a tool that promises a free
|
||||
wide net gets reached for casually and then regretted.
|
||||
|
||||
So this reuses the graduated shape #3851 measured for the push: the top few
|
||||
carry their trigger in full, the rest carry a cut of it. TRUNCATED, never
|
||||
dropped — the trigger is what lets a reader judge relevance without opening
|
||||
the record, and a teaser without one is just an id.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import textwrap
|
||||
import time
|
||||
|
||||
from scribe.mcp._context import current_user_id
|
||||
from scribe.services.embeddings import semantic_search_rules
|
||||
from scribe.services.retrieval_telemetry import record_retrieval
|
||||
|
||||
# The telemetry `source`, and a PULL — never added to AMBIENT_SOURCES (those
|
||||
# are deliveries nobody chose) and never confused with a push arm. Keeping it
|
||||
# separate is load-bearing right now: the push arms' near-miss distributions
|
||||
# are the evidence #4121 rests on, and a pull mixed into them would move the
|
||||
# very numbers that step is arguing from.
|
||||
SOURCE = "wide_net"
|
||||
|
||||
# No bar. That is the point of the tool rather than an oversight — the caller
|
||||
# asked for the wide net precisely because they do not trust a bar to decide
|
||||
# for them here. Every row carries its score, so the reader sees where the
|
||||
# ranking falls off and judges it themselves.
|
||||
THRESHOLD = 0.0
|
||||
|
||||
MAX_LIMIT = 50
|
||||
DEFAULT_LIMIT = 25
|
||||
|
||||
# How many candidates carry their trigger in full before the rest are cut.
|
||||
DEFAULT_DETAIL = 5
|
||||
|
||||
# The cut length for the tail. Long enough to carry the first clause of a
|
||||
# trigger — which is where these state the act they are about — short enough
|
||||
# that forty-five of them stay affordable.
|
||||
_TEASER_CHARS = 140
|
||||
|
||||
|
||||
def _teaser(text: str) -> tuple[str, bool]:
|
||||
"""Shorten at a word break with a visible cut. Returns (text, was_cut).
|
||||
|
||||
The technique is `plugin_context._goal_line`'s, and the fallback is the
|
||||
part worth copying: `textwrap.shorten` returns a bare "…" when the string
|
||||
is one unbroken word longer than the cap, and a raw slice ends mid-word
|
||||
claiming to be the whole thing (#4036). Kept local rather than shared
|
||||
because the two callers wrap it in different sentences; if a third appears,
|
||||
that is the moment to extract it rather than now.
|
||||
"""
|
||||
flat = " ".join((text or "").split())
|
||||
if len(flat) <= _TEASER_CHARS:
|
||||
return flat, False
|
||||
short = textwrap.shorten(flat, width=_TEASER_CHARS, placeholder="…")
|
||||
if short == "…":
|
||||
short = flat[: _TEASER_CHARS - 1] + "…"
|
||||
return short, True
|
||||
|
||||
|
||||
async def what_might_apply(
|
||||
query: str,
|
||||
limit: int = DEFAULT_LIMIT,
|
||||
detail: int = DEFAULT_DETAIL,
|
||||
kind: str = "",
|
||||
project_id: int = 0,
|
||||
) -> dict:
|
||||
"""Every rule that might bear on what you are about to do, ranked, with no bar.
|
||||
|
||||
REACH FOR THIS BEFORE A CONSEQUENTIAL OR IRREVERSIBLE ACT — a push, a
|
||||
merge, a delete, a deploy, anything outward-facing — and before handing
|
||||
work back because you are unsure whether you are allowed to finish it.
|
||||
|
||||
It is the tool the "ask before a consequential act" reflex should call.
|
||||
A rule reaches a session by retrieval, and the arms that push rules at you
|
||||
have a small budget: they deliver the few highest-scoring candidates and
|
||||
say nothing about what sat just underneath. That is right for an arm that
|
||||
fires before every command and wrong for the one moment you actually want
|
||||
to be sure. This is that moment's tool.
|
||||
|
||||
WHAT IT RETURNS, AND WHY THE TAIL LOOKS LIKE NOISE
|
||||
|
||||
There is no threshold. You get the `limit` nearest candidates whatever they
|
||||
score, ordered, each with its score — so the tail IS expected to be
|
||||
irrelevant, and that is the design. You are not reading the list for its
|
||||
average quality; you are reading it for the one record you would otherwise
|
||||
have missed. Scan the triggers, open what looks live with `get_rule(id)`,
|
||||
and ignore the rest.
|
||||
|
||||
The first few carry their trigger in full; the rest carry a cut of it,
|
||||
marked `truncated`. A cut trigger is still enough to decide whether to
|
||||
open the record, which is the whole job of a teaser.
|
||||
|
||||
`kind` is on every row and is never something to infer: a **rule** must be
|
||||
followed, a **preference** records how the operator wants work done.
|
||||
Missing a rule is a mistake; missing a preference costs consistency.
|
||||
|
||||
A NOTE ON WHAT THIS CANNOT DO. It is a pull, so it only helps if you ask.
|
||||
The failure it was built for — a session withholding a routine action
|
||||
because the rule permitting it never arrived — produces no tool call of its
|
||||
own, so nothing will prompt you. Asking is the habit; this is where to put
|
||||
it.
|
||||
|
||||
Args:
|
||||
query: what you are about to do, in the words you would use to
|
||||
describe it — "push to dev after committing", "delete the staging
|
||||
database", "merge dev to main". A command string works; a sentence
|
||||
usually works better, because triggers are written as prose.
|
||||
limit: how many candidates, default 25, capped at 50.
|
||||
detail: how many carry their trigger in FULL before the rest are cut,
|
||||
default 5.
|
||||
kind: "rule" or "preference" to restrict; omit for both.
|
||||
project_id: scope to one project — its own rules plus every global
|
||||
one. Omit to ask the whole rulebook.
|
||||
"""
|
||||
uid = current_user_id()
|
||||
limit = max(1, min(int(limit), MAX_LIMIT))
|
||||
detail = max(0, min(int(detail), limit))
|
||||
|
||||
report: dict = {}
|
||||
started = time.perf_counter()
|
||||
if project_id:
|
||||
raw = await semantic_search_rules(
|
||||
uid, query, limit=limit, threshold=THRESHOLD, kind=kind or None,
|
||||
report=report, project_id=project_id,
|
||||
)
|
||||
else:
|
||||
raw = await semantic_search_rules(
|
||||
uid, query, limit=limit, threshold=THRESHOLD, kind=kind or None,
|
||||
report=report, everywhere=True,
|
||||
)
|
||||
duration_ms = (time.perf_counter() - started) * 1000
|
||||
|
||||
candidates = []
|
||||
for rank, (score, rule) in enumerate(raw):
|
||||
full = rank < detail
|
||||
trigger, was_cut = (
|
||||
(" ".join((rule.when_to_apply or "").split()), False)
|
||||
if full else _teaser(rule.when_to_apply or "")
|
||||
)
|
||||
candidates.append({
|
||||
"id": rule.id,
|
||||
"title": rule.title,
|
||||
# Force, not topic. See the docstring — this is never inferred.
|
||||
"kind": rule.kind,
|
||||
# A rule in a rulebook topic is global; one on a project binds
|
||||
# there alone (milestone 414). Which it is changes how far a
|
||||
# reader should generalise from it.
|
||||
"scope": "project" if rule.project_id else "global",
|
||||
"when_to_apply": trigger,
|
||||
"truncated": was_cut,
|
||||
"score": round(float(score), 4),
|
||||
})
|
||||
|
||||
# Logged as a pull, with `searched` honoured: a search that never ran must
|
||||
# not be recorded as a ranker declining (#3765).
|
||||
record_retrieval(
|
||||
user_id=uid,
|
||||
source=SOURCE,
|
||||
query=query,
|
||||
threshold=THRESHOLD,
|
||||
limit=limit,
|
||||
project_id=project_id or None,
|
||||
is_task=None,
|
||||
results=raw,
|
||||
duration_ms=duration_ms,
|
||||
best_available=report.get("best_available_score"),
|
||||
best_available_id=report.get("best_available_id"),
|
||||
searched=report.get("searched", True),
|
||||
)
|
||||
|
||||
return {
|
||||
"candidates": candidates,
|
||||
"returned": len(candidates),
|
||||
"detailed": min(detail, len(candidates)),
|
||||
# Present so a caller can tell "the corpus offered nothing" from "the
|
||||
# search never ran" — an empty query, an unavailable embedder and a
|
||||
# failed query all return zero rows and mean different things (#3670).
|
||||
"searched": bool(report.get("searched", True)),
|
||||
"open_with": "get_rule(id)",
|
||||
}
|
||||
|
||||
|
||||
def register(mcp) -> None:
|
||||
mcp.tool(name="what_might_apply")(what_might_apply)
|
||||
@@ -75,6 +75,23 @@ class RetrievalTuningEvent(Base):
|
||||
# becomes a formality; the service refuses a blank one.
|
||||
reason: Mapped[str] = mapped_column(Text, nullable=False)
|
||||
|
||||
# WHAT SPACE THIS NUMBER WAS MEASURED IN (#4104). A floor is a cosine
|
||||
# distance in one embedding model's geometry, over documents cut one
|
||||
# particular way — change either and the number describes something that
|
||||
# no longer exists.
|
||||
#
|
||||
# On the EVENT rather than beside the setting, because this table already
|
||||
# holds one row per change and `current_settings` already reads the latest
|
||||
# per dial. A parallel stamp row next to the value would be a second place
|
||||
# to keep in sync, and the two disagreeing is worse than neither.
|
||||
#
|
||||
# Both NULLABLE for the rows written before this step: those were measured
|
||||
# under something, but claiming to know which would be inventing a fact.
|
||||
# Null here means "unstamped", which is a different and honest answer from
|
||||
# naming a model that may be wrong.
|
||||
embedding_model: Mapped[str | None] = mapped_column(Text, nullable=True)
|
||||
shape_version: Mapped[int | None] = mapped_column(Integer, nullable=True)
|
||||
|
||||
__table_args__ = (
|
||||
# The only read this table has: one surface's history, newest first.
|
||||
Index(
|
||||
@@ -94,4 +111,8 @@ class RetrievalTuningEvent(Base):
|
||||
"new_value": self.new_value,
|
||||
"actor": self.actor,
|
||||
"reason": self.reason,
|
||||
# Null for every row written before #4104, and rendered as
|
||||
# "unstamped" rather than guessed at — see the column comments.
|
||||
"embedding_model": self.embedding_model,
|
||||
"shape_version": self.shape_version,
|
||||
}
|
||||
|
||||
@@ -74,8 +74,14 @@ logger = logging.getLogger(__name__)
|
||||
# argument for them silently dropped — and from this step on those dials are
|
||||
# moved by the model, which is exactly the case where the operator needs the
|
||||
# argument to review.
|
||||
# v17 (2026-09) added retrieval_tuning_events.embedding_model / shape_version
|
||||
# (milestone 416 step 6): a floor is a distance in ONE embedding model's
|
||||
# geometry over documents cut one particular way, so the number alone cannot
|
||||
# say whether it still measures anything. Both travel NULLABLE and unfilled —
|
||||
# a row written before the stamp existed restores unstamped, because inventing
|
||||
# the model it was measured under would turn "unknown" into a stated fact.
|
||||
# Bump when the serialized schema changes.
|
||||
BACKUP_VERSION = 16
|
||||
BACKUP_VERSION = 17
|
||||
|
||||
# Every table this backup carries, by its REAL name. Paired with _NOT_INCLUDED
|
||||
# below, these two lists must together account for the entire schema — which is
|
||||
@@ -358,6 +364,12 @@ def _retrieval_tuning_event_rows(rows) -> list[dict]:
|
||||
"user_id": r.user_id, "surface": r.surface, "dial": r.dial,
|
||||
"old_value": r.old_value, "new_value": r.new_value,
|
||||
"actor": r.actor, "reason": r.reason,
|
||||
# Carried, and NOT defaulted to the current model on the way out
|
||||
# (#4104): a row that was unstamped when it was written is still
|
||||
# unstamped after a round trip, and a backup that quietly filled
|
||||
# the gap would turn "we don't know" into a stated fact.
|
||||
"embedding_model": r.embedding_model,
|
||||
"shape_version": r.shape_version,
|
||||
"created_at": r.created_at.isoformat() if r.created_at else None,
|
||||
}
|
||||
for r in rows
|
||||
@@ -1247,6 +1259,12 @@ async def _restore_v2(data: dict) -> dict:
|
||||
new_value=t_data.get("new_value"),
|
||||
actor=t_data.get("actor") or "model",
|
||||
reason=t_data.get("reason", ""),
|
||||
# .get with no default, deliberately (v17): an archive written
|
||||
# before the stamp existed has no key here, and None is the
|
||||
# right answer for it — the same "unstamped" a pre-#4104 row
|
||||
# carries in place.
|
||||
embedding_model=t_data.get("embedding_model"),
|
||||
shape_version=t_data.get("shape_version"),
|
||||
created_at=_dt(t_data.get("created_at")),
|
||||
))
|
||||
stats["retrieval_tuning_events"] += 1
|
||||
|
||||
@@ -217,6 +217,38 @@ def embedding_text(title: str | None, body: str | None) -> str:
|
||||
# wipe migrations 0067/0077 had to do.
|
||||
CHUNKER_VERSION = 1
|
||||
|
||||
|
||||
# The public name of the space every score lives in, and the two facts that
|
||||
# can invalidate a tuned number (#4104).
|
||||
#
|
||||
# EMBEDDING_MODEL is `_MODEL_NAME` under a name other modules may read. It was
|
||||
# private until this step, which is precisely why nothing outside this file
|
||||
# could state what space a threshold was measured in — a floor is a distance in
|
||||
# THIS model's geometry and means nothing in another's.
|
||||
#
|
||||
# The pair is what a stamp is made of, and the pairing is the point: a score
|
||||
# changes when the model changes (different geometry) OR when the document
|
||||
# shape changes (different text embedded for the same record). Either one
|
||||
# invalidates a number that was measured before it.
|
||||
EMBEDDING_MODEL = _MODEL_NAME
|
||||
|
||||
|
||||
def calibration_stamp() -> dict:
|
||||
"""What a tuned retrieval number was measured against.
|
||||
|
||||
ONE definition, because the alternative is each reader assembling the pair
|
||||
and one of them forgetting a half. Returned as a dict rather than a string
|
||||
so a mismatch can say WHICH half moved — "the model changed" and "the
|
||||
chunker changed" call for different responses, and a fused string can only
|
||||
report that something did.
|
||||
|
||||
Deliberately says nothing about the CHAT model. A Claude upgrade changes no
|
||||
score here and must never raise a recalibration prompt: a false alarm on
|
||||
this surface teaches an operator to ignore the true one. `tests/
|
||||
test_calibration_stamp.py` asserts that absence rather than trusting it.
|
||||
"""
|
||||
return {"embedding_model": EMBEDDING_MODEL, "shape_version": CHUNKER_VERSION}
|
||||
|
||||
# Character budget approximating the model window. Tokens-per-char varies by
|
||||
# content — ~4 chars/token for prose, closer to 3 for code and tables — so 1400
|
||||
# chars sits at roughly 350-470 tokens, leaving headroom for the title prefixed
|
||||
|
||||
@@ -0,0 +1,248 @@
|
||||
"""Carrying a tuned floor across an embedding-model change (#4104).
|
||||
|
||||
THE PROBLEM
|
||||
|
||||
A floor is a cosine similarity, and a cosine similarity is a distance in one
|
||||
model's geometry. `bge-small-en-v1.5` → `bge-base-en-v1.5` moves every score on
|
||||
the install at once, in no direction anyone can predict per record. The numbers
|
||||
in `settings` survive the swap unchanged and silently stop describing anything:
|
||||
the bar keeps applying, the telemetry keeps filling, and nothing anywhere says
|
||||
the arm is now cutting in a different place.
|
||||
|
||||
Re-deriving six floors by hand is the alternative, and it is the thing the
|
||||
operator asked for a way out of — *"a path for thresholds to be inherited by
|
||||
the next version or different model so that they don't have to recalibrate a
|
||||
lot."*
|
||||
|
||||
WHAT TRANSFERS, AND WHY IT IS NOT THE NUMBER
|
||||
|
||||
The raw cosine does not transfer. **The percentile it represented does.** A
|
||||
floor's real content is a decision about SELECTIVITY — "admit roughly the top
|
||||
fifth of what this arm sees" — and that decision is about the operator's
|
||||
tolerance for noise on that surface, not about the embedder. It was true before
|
||||
the model changed and is still true after.
|
||||
|
||||
So: measure what fraction of this surface's calls the old floor admitted, using
|
||||
the scores the old model actually produced; re-score the same queries under the
|
||||
new one; and take the value that admits the same fraction. Same decision, new
|
||||
units.
|
||||
|
||||
WHY `best_available_score` IS THE FIGURE ON BOTH SIDES
|
||||
|
||||
Because it is the one number that exists whether or not a call returned
|
||||
anything — the highest score the corpus offered, before the bar was applied
|
||||
(#3670). It is what `retrieval_logs` recorded under the old model, and it is
|
||||
what a re-score reproduces under the new one, so the two sides are the same
|
||||
measurement rather than two things that resemble each other.
|
||||
|
||||
It also makes the re-score cheap to get right: `best_available_score` is
|
||||
computed over the whole candidate set BEFORE `limit` and before any
|
||||
`exclude_ids` the arm passed, so neither has to be reproduced here. Only the
|
||||
corpus FILTERS matter, which is why `_RESCORERS` below carries those and
|
||||
nothing else.
|
||||
|
||||
WHAT THIS DOES NOT DO
|
||||
|
||||
It does not fire by itself, and `migrate_floor` will not write anything unless
|
||||
asked twice — `apply=True` on top of having read the dry run. A model change is
|
||||
exactly the moment when every number is uncertain at once, which is the worst
|
||||
possible moment to let a statistic move six dials unattended. The percentile is
|
||||
a starting point on the new scale, in the same sense the shipped defaults are a
|
||||
starting point: better than a stale number, not a substitute for reading what
|
||||
the surface actually turned away.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
|
||||
from sqlalchemy import select
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.retrieval_log import RetrievalLog
|
||||
from scribe.services.embeddings import (
|
||||
calibration_stamp,
|
||||
semantic_search_notes,
|
||||
semantic_search_rules,
|
||||
)
|
||||
from scribe.services.retrieval_surfaces import floor_for, get_surface
|
||||
from scribe.services.retrieval_tuning import set_dial
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# How many of a surface's recent calls to re-score. Every one is an embedding
|
||||
# plus a full scan, so this is a real cost — but a percentile off twenty calls
|
||||
# is noise, and the arms that matter here log hundreds a week.
|
||||
DEFAULT_SAMPLE = 200
|
||||
|
||||
|
||||
async def _rescore_rules(user_id: int, query: str, project_id: int | None,
|
||||
kind: str | None) -> float | None:
|
||||
rep: dict = {}
|
||||
await semantic_search_rules(
|
||||
user_id, query, limit=1, threshold=0.0, kind=kind,
|
||||
report=rep, project_id=project_id or None,
|
||||
)
|
||||
return rep.get("best_available_score")
|
||||
|
||||
|
||||
async def _rescore_notes(user_id: int, query: str, project_id: int | None,
|
||||
note_type, task_kind) -> float | None:
|
||||
rep: dict = {}
|
||||
await semantic_search_notes(
|
||||
user_id, query, limit=1, threshold=0.0,
|
||||
project_id=project_id or None, note_type=note_type,
|
||||
task_kind=task_kind, scope="browse", report=rep,
|
||||
)
|
||||
return rep.get("best_available_score")
|
||||
|
||||
|
||||
# ONE re-scorer per surface, carrying that arm's corpus filters and nothing
|
||||
# else. These filters are stated a second time here — the arms in
|
||||
# `plugin_context` are where they are first declared — and that duplication is
|
||||
# deliberate rather than overlooked: the alternative is calling the arms
|
||||
# themselves, which build a menu, write telemetry and record records as
|
||||
# surfaced. A migration that logged two hundred fake retrievals would corrupt
|
||||
# the very table the next tuning decision reads.
|
||||
#
|
||||
# `tests/test_retrieval_migration.py` asserts every registry surface has an
|
||||
# entry, so a seventh arm cannot quietly become un-migratable.
|
||||
_RESCORERS = {
|
||||
"auto_inject": lambda u, q, p: _rescore_notes(u, q, p, None, None),
|
||||
"write_path": lambda u, q, p: _rescore_notes(
|
||||
u, q, p, ("snippet", "note"), "issue"
|
||||
),
|
||||
"write_path_rule": lambda u, q, p: _rescore_rules(u, q, p, None),
|
||||
"pre_tool_rule": lambda u, q, p: _rescore_rules(u, q, p, None),
|
||||
"prompt_rule": lambda u, q, p: _rescore_rules(u, q, p, None),
|
||||
"report_preference": lambda u, q, p: _rescore_rules(u, q, p, "preference"),
|
||||
}
|
||||
|
||||
|
||||
def _floor_admitting(scores: list[float], fraction: float) -> float:
|
||||
"""The floor that admits `fraction` of `scores`, on this scale.
|
||||
|
||||
Deliberately exact rather than interpolated: with the scores sorted
|
||||
highest-first, the k-th one IS the bar that admits exactly k. An
|
||||
interpolated quantile would return a number no observed call sits on, which
|
||||
is harder to sanity-check against the sample it came from.
|
||||
|
||||
A fraction rounding to zero returns a floor just above the best score seen —
|
||||
an arm that admitted nothing keeps admitting nothing, rather than being
|
||||
quietly reopened by a migration.
|
||||
"""
|
||||
ranked = sorted(scores, reverse=True)
|
||||
k = int(round(fraction * len(ranked)))
|
||||
if k <= 0:
|
||||
return min(1.0, ranked[0] + 1e-6)
|
||||
return ranked[min(k, len(ranked)) - 1]
|
||||
|
||||
|
||||
async def migrate_floor(
|
||||
user_id: int,
|
||||
surface: str,
|
||||
*,
|
||||
sample: int = DEFAULT_SAMPLE,
|
||||
apply: bool = False,
|
||||
) -> dict:
|
||||
"""Recompute one surface's floor on the current model, preserving selectivity.
|
||||
|
||||
Returns the working: how many calls were sampled, what fraction the old
|
||||
floor admitted, and what value admits the same fraction now. Writes nothing
|
||||
unless `apply=True`, and when it does it writes an ordinary tuning event
|
||||
with the arithmetic in its reason — a migrated floor is reviewable and
|
||||
revertible on exactly the same terms as one a reader chose.
|
||||
"""
|
||||
get_surface(surface) # refuses an unknown name
|
||||
rescore = _RESCORERS.get(surface)
|
||||
if rescore is None:
|
||||
raise ValueError(
|
||||
f"no re-scorer for surface {surface!r}. A surface that cannot be "
|
||||
"re-scored cannot be migrated — add it to _RESCORERS beside the "
|
||||
"arm's own corpus filters."
|
||||
)
|
||||
|
||||
old_floor = await floor_for(user_id, surface)
|
||||
async with async_session() as session:
|
||||
rows = (await session.execute(
|
||||
select(
|
||||
RetrievalLog.query,
|
||||
RetrievalLog.project_id,
|
||||
RetrievalLog.best_available_score,
|
||||
)
|
||||
.where(
|
||||
RetrievalLog.source == surface,
|
||||
RetrievalLog.user_id == user_id,
|
||||
RetrievalLog.best_available_score.is_not(None),
|
||||
RetrievalLog.query.is_not(None),
|
||||
RetrievalLog.query != "",
|
||||
)
|
||||
.order_by(RetrievalLog.id.desc())
|
||||
.limit(max(1, int(sample)))
|
||||
)).all()
|
||||
|
||||
if not rows:
|
||||
# Not an error. A surface with no logged calls has no evidence of what
|
||||
# its floor was doing, and inventing a migration for it would be the
|
||||
# exact failure this module's docstring warns about.
|
||||
return {
|
||||
"surface": surface, "migrated": False,
|
||||
"why": "no logged calls carry a best_available_score for this "
|
||||
"surface, so there is no old distribution to preserve",
|
||||
"sampled": 0, "old_floor": old_floor,
|
||||
}
|
||||
|
||||
old_scores = [float(r.best_available_score) for r in rows]
|
||||
admitted = sum(1 for s in old_scores if s >= old_floor)
|
||||
fraction = admitted / len(old_scores)
|
||||
|
||||
new_scores: list[float] = []
|
||||
for r in rows:
|
||||
score = await rescore(user_id, r.query, r.project_id)
|
||||
if score is not None:
|
||||
new_scores.append(float(score))
|
||||
|
||||
if not new_scores:
|
||||
# The corpus answered nothing for any sampled query. Almost always an
|
||||
# embedder that has not finished backfilling under the new model —
|
||||
# migrating from it would set every floor off an empty distribution.
|
||||
return {
|
||||
"surface": surface, "migrated": False,
|
||||
"why": "re-scoring returned nothing for any sampled query — the "
|
||||
"corpus is probably not embedded under the current model yet",
|
||||
"sampled": len(rows), "old_floor": old_floor,
|
||||
"old_admit_rate": round(fraction, 4),
|
||||
}
|
||||
|
||||
proposed = round(min(1.0, max(0.0, _floor_admitting(new_scores, fraction))), 4)
|
||||
stamp = calibration_stamp()
|
||||
reason = (
|
||||
f"Migrated across a calibration change, preserving selectivity: the old "
|
||||
f"floor {old_floor} admitted {admitted} of {len(old_scores)} sampled "
|
||||
f"calls ({fraction:.1%}); {proposed} admits the same share of "
|
||||
f"{len(new_scores)} queries re-scored under "
|
||||
f"{stamp['embedding_model']}/shape {stamp['shape_version']}. The "
|
||||
f"percentile is what carried across, not the number — spot-check the "
|
||||
f"arm before trusting it."
|
||||
)
|
||||
|
||||
result = {
|
||||
"surface": surface,
|
||||
"migrated": False,
|
||||
"sampled": len(rows),
|
||||
"rescored": len(new_scores),
|
||||
"old_floor": old_floor,
|
||||
"old_admit_rate": round(fraction, 4),
|
||||
"old_score_range": [round(min(old_scores), 4), round(max(old_scores), 4)],
|
||||
"new_score_range": [round(min(new_scores), 4), round(max(new_scores), 4)],
|
||||
"proposed_floor": proposed,
|
||||
"calibration": stamp,
|
||||
"reason": reason,
|
||||
}
|
||||
if not apply:
|
||||
return result
|
||||
|
||||
result["migrated"] = True
|
||||
result["applied"] = await set_dial(
|
||||
user_id, surface, "floor", proposed, reason=reason, actor="model",
|
||||
)
|
||||
return result
|
||||
@@ -103,6 +103,31 @@ class Surface:
|
||||
asks: str
|
||||
over: str
|
||||
fires: str
|
||||
measured_model: str = "BAAI/bge-small-en-v1.5"
|
||||
measured_shape: int = 1
|
||||
"""What the SHIPPED defaults above were measured against (#4104).
|
||||
|
||||
A floor is a distance in one embedding model's geometry, over documents cut
|
||||
one particular way. Either can change, and when one does every number in
|
||||
this table describes something that no longer exists.
|
||||
|
||||
TWO FIELDS, NEVER ONE FUSED STRING (rule 149). A mismatch has to be able to
|
||||
say WHICH half moved: a new embedding model and a re-cut document shape
|
||||
invalidate the same numbers for different reasons and call for different
|
||||
responses. `"<model>@<n>"` could only report that something changed, which
|
||||
is the answer nobody can act on. Same reason `calibration_stamp()` returns
|
||||
a dict and the event table gives each half its own column.
|
||||
|
||||
Recorded per surface rather than once for the module because they need not
|
||||
move together: a surface retuned after a model change carries the new stamp
|
||||
while its untouched siblings still carry the old one, and telling those
|
||||
apart is the whole job.
|
||||
|
||||
LITERALS, deliberately, rather than an import of the live values — a stamp
|
||||
says what was true when the number was chosen, so one that tracked the
|
||||
current model would always agree with it and could never report staleness.
|
||||
"""
|
||||
|
||||
budget_falls_back_to: str = ""
|
||||
"""A budget key to inherit when this surface has none of its own set.
|
||||
|
||||
|
||||
@@ -47,6 +47,7 @@ from sqlalchemy import select
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.retrieval_tuning import RetrievalTuningEvent
|
||||
from scribe.services.embeddings import calibration_stamp
|
||||
from scribe.services.retrieval_surfaces import (
|
||||
MAX_BUDGET,
|
||||
budget_for,
|
||||
@@ -86,6 +87,50 @@ def _clean_reason(reason: str) -> str:
|
||||
return text
|
||||
|
||||
|
||||
def _calibration(row, s, live: dict) -> dict:
|
||||
"""What space one dial's number was chosen in, and whether that space moved.
|
||||
|
||||
Three sources, and they are not interchangeable:
|
||||
|
||||
- `tuned` — the dial was moved after #4104 and the event carries its
|
||||
stamp. The only case where the answer is known.
|
||||
- `shipped` — the dial has never been moved, so the number in force is
|
||||
the registry default, and the registry records what THAT
|
||||
was measured against (`Surface.measured_model`).
|
||||
- `unstamped` — the dial was moved before this step existed. It was
|
||||
measured under something; naming it would be inventing a
|
||||
fact, so `stale` is None rather than True or False.
|
||||
"Unknown" and "fine" must not render the same.
|
||||
|
||||
`model_changed` and `shape_changed` are reported apart (rule 149) because
|
||||
they call for different responses: a new embedding model means every number
|
||||
is a distance in a geometry that no longer exists, while a re-cut document
|
||||
shape means the same records now embed different text. A caller that only
|
||||
ever sees `stale: true` cannot tell those apart.
|
||||
"""
|
||||
if row is None:
|
||||
model, shape, source = s.measured_model, s.measured_shape, "shipped"
|
||||
elif row.embedding_model is None and row.shape_version is None:
|
||||
return {
|
||||
"source": "unstamped", "embedding_model": None,
|
||||
"shape_version": None, "model_changed": None,
|
||||
"shape_changed": None, "stale": None,
|
||||
}
|
||||
else:
|
||||
model, shape, source = row.embedding_model, row.shape_version, "tuned"
|
||||
|
||||
model_changed = model != live["embedding_model"]
|
||||
shape_changed = shape != live["shape_version"]
|
||||
return {
|
||||
"source": source,
|
||||
"embedding_model": model,
|
||||
"shape_version": shape,
|
||||
"model_changed": model_changed,
|
||||
"shape_changed": shape_changed,
|
||||
"stale": model_changed or shape_changed,
|
||||
}
|
||||
|
||||
|
||||
async def current_settings(user_id: int) -> list[dict]:
|
||||
"""Every tunable surface with its live pair and its last stated reason.
|
||||
|
||||
@@ -94,23 +139,44 @@ async def current_settings(user_id: int) -> list[dict]:
|
||||
often — because a floor cannot be moved sensibly without those three — and
|
||||
the reason last given, so the next change argues with the last one instead
|
||||
of overwriting it blind.
|
||||
|
||||
From #4104 it also carries `calibration` per dial: the embedding model and
|
||||
document shape the number in force was measured in, and whether either has
|
||||
moved since. NOTHING AUTO-RETUNES on the strength of it. A stale stamp says
|
||||
a number is a measurement of a space that no longer exists — it does not
|
||||
say what the number should be now, and the one time a statistic was allowed
|
||||
to answer that question it was wrong (see the module docstring). The stamp
|
||||
is here so a reader knows which floors to go and re-measure.
|
||||
"""
|
||||
live = calibration_stamp()
|
||||
out: list[dict] = []
|
||||
async with async_session() as session:
|
||||
for name in surface_names():
|
||||
s = get_surface(name)
|
||||
rows = (
|
||||
# ONE QUERY PER DIAL, not one `limit(len(DIALS))` over both.
|
||||
# "The newest two rows" is not "the newest row of each kind": a
|
||||
# surface whose floor was moved three times and whose budget was
|
||||
# moved once returns two floor rows, and the budget change
|
||||
# disappears. That was a missing reason when this only fed
|
||||
# `last_change`; since #4104 it is also a WRONG calibration answer —
|
||||
# a tuned dial reporting as "still on the shipped default", which is
|
||||
# the one state a reader would not think to check.
|
||||
last = {}
|
||||
for dial in DIALS:
|
||||
row = (
|
||||
await session.execute(
|
||||
select(RetrievalTuningEvent)
|
||||
.where(
|
||||
RetrievalTuningEvent.surface == name,
|
||||
RetrievalTuningEvent.user_id == user_id,
|
||||
RetrievalTuningEvent.dial == dial,
|
||||
)
|
||||
.order_by(RetrievalTuningEvent.created_at.desc())
|
||||
.limit(len(DIALS))
|
||||
.limit(1)
|
||||
)
|
||||
).scalars().all()
|
||||
last = {r.dial: r for r in rows}
|
||||
).scalars().first()
|
||||
if row is not None:
|
||||
last[dial] = row
|
||||
out.append({
|
||||
"surface": name,
|
||||
"floor": await floor_for(user_id, name),
|
||||
@@ -126,6 +192,13 @@ async def current_settings(user_id: int) -> list[dict]:
|
||||
"last_change": {
|
||||
dial: last[dial].to_dict() for dial in DIALS if dial in last
|
||||
},
|
||||
# Always present for BOTH dials, unlike `last_change`: an
|
||||
# untouched dial still has a number in force, and that number
|
||||
# was still measured in some space. Reported, never acted on —
|
||||
# see the note below on why nothing auto-retunes.
|
||||
"calibration": {
|
||||
dial: _calibration(last.get(dial), s, live) for dial in DIALS
|
||||
},
|
||||
})
|
||||
return out
|
||||
|
||||
@@ -179,9 +252,16 @@ async def set_dial(
|
||||
|
||||
await set_setting(user_id, key, stored)
|
||||
async with async_session() as session:
|
||||
# Stamped with the space this number was chosen in (#4104). Read at
|
||||
# write time rather than passed in: the caller measuring a floor and
|
||||
# the caller recording it are the same call, so there is no window in
|
||||
# which they could disagree.
|
||||
stamp = calibration_stamp()
|
||||
session.add(RetrievalTuningEvent(
|
||||
user_id=user_id, surface=surface, dial=dial,
|
||||
old_value=old, new_value=applied, actor=actor, reason=text,
|
||||
embedding_model=stamp["embedding_model"],
|
||||
shape_version=stamp["shape_version"],
|
||||
))
|
||||
await session.commit()
|
||||
|
||||
|
||||
@@ -0,0 +1,238 @@
|
||||
"""A tuned number carries the space it was measured in — and only that (#4104).
|
||||
|
||||
WHY THIS EXISTS
|
||||
|
||||
Milestone 416 step 6 answers *"a path for thresholds to be inherited by the next
|
||||
version or different model so that they don't have to recalibrate a lot."* The
|
||||
mechanism is a stamp: every tuning event records the embedding model and
|
||||
document shape the number was chosen under, and a mismatch is reported rather
|
||||
than left to be noticed.
|
||||
|
||||
THE ONE THAT MATTERS MOST IS THE ABSENCE
|
||||
|
||||
`test_no_chat_model_identifier_reaches_the_calibration_path` is the load-bearing
|
||||
guard here, and it is a guard against a plausible mistake rather than a
|
||||
hypothetical one. These floors live in BAAI/bge-small-en-v1.5's vector space.
|
||||
The CHAT model — Claude — produces none of these scores and changes none of
|
||||
them, so a Claude upgrade must trigger nothing at all. If it ever did, the
|
||||
operator would be asked to recalibrate six dials for no reason, learn that this
|
||||
surface cries wolf, and ignore it on the day `bge-small` → `bge-base` actually
|
||||
moves every score on the install. A false alarm here does not cost a
|
||||
notification; it costs the real alarm.
|
||||
|
||||
The rest pins the honest-unknown behaviour: a row written before stamps existed
|
||||
reports as `unstamped` with `stale: None`, never as fine and never as a model
|
||||
name somebody guessed at.
|
||||
"""
|
||||
import re
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import embeddings as emb
|
||||
from scribe.services import retrieval_tuning as rt
|
||||
from scribe.services.retrieval_surfaces import SURFACES, get_surface
|
||||
from tests.helpers import make_mock_session, tool_doc
|
||||
|
||||
# Names for the thing that talks, as opposed to the thing that embeds. Any of
|
||||
# these appearing in the calibration path means a chat-model change could move a
|
||||
# stamp — see the module docstring for why that is the expensive failure.
|
||||
_CHAT_MODEL_WORDS = (
|
||||
"claude", "opus", "sonnet", "haiku", "gpt", "llama", "mistral", "gemini",
|
||||
"anthropic", "openai", "chat_model", "chatmodel", "model_version",
|
||||
)
|
||||
|
||||
|
||||
def test_calibration_stamp_names_the_embedder_and_the_shape():
|
||||
stamp = emb.calibration_stamp()
|
||||
assert stamp == {
|
||||
"embedding_model": emb.EMBEDDING_MODEL,
|
||||
"shape_version": emb.CHUNKER_VERSION,
|
||||
}
|
||||
# Two keys, never one fused string (rule 149): a mismatch has to be able to
|
||||
# say WHICH half moved, because a new embedder and a re-cut document
|
||||
# invalidate the same numbers for different reasons.
|
||||
assert len(stamp) == 2
|
||||
|
||||
|
||||
def test_the_stamp_tracks_the_embedding_model_constant_not_a_copy_of_it():
|
||||
"""A second literal would drift, and drift here reads as a model change."""
|
||||
with patch.object(emb, "EMBEDDING_MODEL", "some/other-model"):
|
||||
assert emb.calibration_stamp()["embedding_model"] == "some/other-model"
|
||||
with patch.object(emb, "CHUNKER_VERSION", 99):
|
||||
assert emb.calibration_stamp()["shape_version"] == 99
|
||||
|
||||
|
||||
def test_no_chat_model_identifier_reaches_the_calibration_path():
|
||||
"""THE GUARD. A Claude upgrade must move nothing in this path.
|
||||
|
||||
Structural rather than behavioural on purpose: the failure is someone
|
||||
*adding* a chat-model field in good faith ("surely the model matters"), and
|
||||
no behavioural test catches a field that has not been written yet. Asserted
|
||||
over the source of every module that produces or consumes a stamp.
|
||||
"""
|
||||
import inspect
|
||||
|
||||
from scribe.services import retrieval_migration as rm
|
||||
|
||||
for label, obj in (
|
||||
("calibration_stamp", emb.calibration_stamp),
|
||||
("_calibration", rt._calibration),
|
||||
("migrate_floor", rm.migrate_floor),
|
||||
):
|
||||
src = inspect.getsource(obj).lower()
|
||||
# Comments and docstrings here legitimately discuss the chat model in
|
||||
# order to rule it out, so strip prose and assert on code only.
|
||||
code = "\n".join(
|
||||
line.split("#", 1)[0]
|
||||
for line in re.sub(r'""".*?"""', "", src, flags=re.S).splitlines()
|
||||
)
|
||||
for word in _CHAT_MODEL_WORDS:
|
||||
assert word not in code, f"{label} names the chat model: {word!r}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_moved_dial_is_written_with_the_live_stamp():
|
||||
session = make_mock_session()
|
||||
with patch.object(rt, "async_session", MagicMock(return_value=session)), \
|
||||
patch.object(rt, "set_setting", AsyncMock()), \
|
||||
patch.object(rt, "floor_for", AsyncMock(return_value=0.72)), \
|
||||
patch.object(rt, "budget_for", AsyncMock(return_value=3)), \
|
||||
patch.object(rt, "calibration_stamp",
|
||||
MagicMock(return_value={"embedding_model": "m/x",
|
||||
"shape_version": 7})):
|
||||
await rt.set_dial(
|
||||
1, "prompt_rule", "floor", 0.66,
|
||||
reason="read the five refused records; four were genuine matches",
|
||||
)
|
||||
event = session.add.call_args[0][0]
|
||||
assert event.embedding_model == "m/x"
|
||||
assert event.shape_version == 7
|
||||
|
||||
|
||||
def _row(model="BAAI/bge-small-en-v1.5", shape=1):
|
||||
return MagicMock(embedding_model=model, shape_version=shape)
|
||||
|
||||
|
||||
def test_an_untouched_dial_reports_the_shipped_default_it_is_still_on():
|
||||
s = get_surface("prompt_rule")
|
||||
live = {"embedding_model": s.measured_model, "shape_version": s.measured_shape}
|
||||
cal = rt._calibration(None, s, live)
|
||||
assert cal["source"] == "shipped"
|
||||
assert cal["stale"] is False
|
||||
|
||||
|
||||
def test_a_dial_moved_before_stamps_existed_is_unknown_not_fine():
|
||||
"""`stale: None`, because "we don't know" and "it's fine" are not the same.
|
||||
|
||||
Collapsing them would hide the dials MOST likely to be wrong — the ones
|
||||
somebody tuned longest ago.
|
||||
"""
|
||||
s = get_surface("prompt_rule")
|
||||
cal = rt._calibration(_row(model=None, shape=None), s, emb.calibration_stamp())
|
||||
assert cal["source"] == "unstamped"
|
||||
assert cal["stale"] is None
|
||||
assert cal["model_changed"] is None and cal["shape_changed"] is None
|
||||
|
||||
|
||||
def test_a_model_change_and_a_shape_change_are_reported_apart():
|
||||
"""Rule 149. Two conditions, two answers — they call for different work."""
|
||||
s = get_surface("prompt_rule")
|
||||
live = {"embedding_model": "new/model", "shape_version": 1}
|
||||
cal = rt._calibration(_row(model="old/model", shape=1), s, live)
|
||||
assert (cal["model_changed"], cal["shape_changed"], cal["stale"]) == (
|
||||
True, False, True,
|
||||
)
|
||||
|
||||
live = {"embedding_model": "old/model", "shape_version": 2}
|
||||
cal = rt._calibration(_row(model="old/model", shape=1), s, live)
|
||||
assert (cal["model_changed"], cal["shape_changed"], cal["stale"]) == (
|
||||
False, True, True,
|
||||
)
|
||||
|
||||
|
||||
def test_a_dial_tuned_under_the_live_stamp_is_not_stale():
|
||||
s = get_surface("prompt_rule")
|
||||
live = emb.calibration_stamp()
|
||||
cal = rt._calibration(
|
||||
_row(model=live["embedding_model"], shape=live["shape_version"]), s, live,
|
||||
)
|
||||
assert cal["source"] == "tuned"
|
||||
assert cal["stale"] is False
|
||||
|
||||
|
||||
def test_every_surface_ships_a_stamp_for_its_defaults():
|
||||
"""A default with no stamp cannot be told from one measured yesterday."""
|
||||
for name, s in SURFACES.items():
|
||||
assert s.measured_model, f"{name} default has no measured model"
|
||||
assert isinstance(s.measured_shape, int), f"{name} shape is not a version"
|
||||
|
||||
|
||||
def test_the_registry_stamp_is_a_literal_not_the_live_value():
|
||||
"""It says what WAS true, so it must not follow the current constants.
|
||||
|
||||
A field that tracked `EMBEDDING_MODEL` would agree with it forever and could
|
||||
never report the one thing it exists to report.
|
||||
"""
|
||||
with patch.object(emb, "EMBEDDING_MODEL", "some/other-model"):
|
||||
assert get_surface("prompt_rule").measured_model != "some/other-model"
|
||||
|
||||
|
||||
def test_the_tool_says_nothing_auto_retunes():
|
||||
"""The contract an agent reads. A stale stamp is a prompt to MEASURE.
|
||||
|
||||
Without this, the obvious next move on seeing `stale: true` is to move the
|
||||
dial — which is tuning from a statistic, the exact failure #4102 measured
|
||||
pointing the wrong way.
|
||||
"""
|
||||
doc = tool_doc("scribe.mcp.tools.retrieval_tuning", "retrieval_surfaces")
|
||||
assert "calibration" in doc
|
||||
assert "NOTHING IS RETUNED AUTOMATICALLY" in doc
|
||||
assert "unstamped" in doc
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_dial_is_read_per_dial_not_from_the_newest_two_rows():
|
||||
"""The floor's history must not be able to bury the budget's.
|
||||
|
||||
"The newest two rows" and "the newest row of each dial" differ the moment
|
||||
one dial moves more often than the other — which is the normal case, since
|
||||
floors get walked and budgets rarely do. Before this was one query per dial,
|
||||
a surface with three floor changes and one budget change reported the budget
|
||||
as untouched: a WRONG calibration answer rather than a missing one, and
|
||||
wrong in the direction a reader would not think to check.
|
||||
"""
|
||||
session = make_mock_session()
|
||||
per_dial = {
|
||||
"floor": MagicMock(dial="floor", embedding_model="old/model",
|
||||
shape_version=1,
|
||||
to_dict=MagicMock(return_value={"dial": "floor"})),
|
||||
"budget": MagicMock(dial="budget", embedding_model="old/model",
|
||||
shape_version=1,
|
||||
to_dict=MagicMock(return_value={"dial": "budget"})),
|
||||
}
|
||||
calls = []
|
||||
|
||||
def execute(stmt):
|
||||
# The dial is whichever the WHERE clause names; a query that did not
|
||||
# scope by dial would render this stand-in unable to answer, which is
|
||||
# the point.
|
||||
sql = str(stmt.compile(compile_kwargs={"literal_binds": True}))
|
||||
dial = "budget" if "'budget'" in sql else "floor"
|
||||
calls.append(dial)
|
||||
result = MagicMock()
|
||||
result.scalars.return_value.first.return_value = per_dial[dial]
|
||||
return result
|
||||
|
||||
session.execute = AsyncMock(side_effect=execute)
|
||||
with patch.object(rt, "async_session", MagicMock(return_value=session)), \
|
||||
patch.object(rt, "floor_for", AsyncMock(return_value=0.7)), \
|
||||
patch.object(rt, "budget_for", AsyncMock(return_value=3)):
|
||||
out = await rt.current_settings(1)
|
||||
|
||||
assert calls.count("floor") == len(SURFACES)
|
||||
assert calls.count("budget") == len(SURFACES)
|
||||
for row in out:
|
||||
# Both dials tuned under a model that is not the live one.
|
||||
assert row["calibration"]["budget"]["source"] == "tuned"
|
||||
assert row["calibration"]["budget"]["stale"] is True
|
||||
@@ -0,0 +1,159 @@
|
||||
"""Carrying a floor across a calibration change (#4104).
|
||||
|
||||
WHAT THIS PINS
|
||||
|
||||
1. **The percentile is what transfers, not the number.** A floor's content is
|
||||
a decision about selectivity; the cosine expressing it is units. So a
|
||||
migration that admitted 30% before admits 30% after, whatever the new
|
||||
scores look like.
|
||||
2. **Nothing is written unless asked twice.** `apply` defaults to False. A
|
||||
model change makes every number uncertain at once, which is the worst
|
||||
moment to let a statistic move six dials unattended.
|
||||
3. **Missing evidence is a refusal, not a guess.** No logged calls, or a
|
||||
corpus that re-scores to nothing, returns `migrated: False` with a reason —
|
||||
never a floor computed off an empty distribution, which is how an install
|
||||
mid-backfill would end up with every bar at zero.
|
||||
4. **Every registry surface can be migrated.** A seventh arm that nobody adds
|
||||
a re-scorer for is one whose floor silently cannot survive a model change.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.services import retrieval_migration as rm
|
||||
from scribe.services.retrieval_surfaces import SURFACES
|
||||
from tests.helpers import make_mock_session, tool_doc
|
||||
|
||||
|
||||
def _logs(pairs):
|
||||
"""Rows as `migrate_floor` reads them: (query, project_id, old score)."""
|
||||
return [MagicMock(query=q, project_id=p, best_available_score=s)
|
||||
for q, p, s in pairs]
|
||||
|
||||
|
||||
def _session_with(rows):
|
||||
"""`.all()` is SYNCHRONOUS on a Result, so it needs a MagicMock.
|
||||
|
||||
`make_mock_session` is an AsyncMock, and every child of an AsyncMock is one
|
||||
too — leaving `.all` as it comes hands the service a coroutine where it
|
||||
expects a list, the same trap the helper's docstring flags for `add`.
|
||||
"""
|
||||
session = make_mock_session()
|
||||
session.execute.return_value = MagicMock(all=MagicMock(return_value=rows))
|
||||
return session
|
||||
|
||||
|
||||
def test_every_surface_has_a_rescorer():
|
||||
"""Otherwise a surface's floor cannot cross a model change at all."""
|
||||
assert set(rm._RESCORERS) == set(SURFACES)
|
||||
|
||||
|
||||
def test_the_floor_that_admits_a_fraction_is_an_observed_score():
|
||||
scores = [0.9, 0.8, 0.7, 0.6, 0.5, 0.4, 0.3, 0.2, 0.1, 0.05]
|
||||
# 30% of ten is three; the third-best score is the bar that admits exactly
|
||||
# those three. Exact rather than interpolated, so the answer can be checked
|
||||
# against the sample it came from.
|
||||
assert rm._floor_admitting(scores, 0.3) == 0.7
|
||||
assert sum(1 for s in scores if s >= 0.7) == 3
|
||||
|
||||
|
||||
def test_a_surface_that_admitted_nothing_keeps_admitting_nothing():
|
||||
"""A migration must not quietly reopen an arm the operator had shut."""
|
||||
scores = [0.5, 0.4, 0.3]
|
||||
assert rm._floor_admitting(scores, 0.0) > max(scores)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_selectivity_is_preserved_across_a_scale_change():
|
||||
"""The whole idea, on numbers that move a long way.
|
||||
|
||||
Old scores cluster near 0.7 with a bar at 0.72 admitting two of five. New
|
||||
scores sit far lower — a different geometry — and the proposal is the value
|
||||
that admits two of five there, not anything resembling 0.72.
|
||||
"""
|
||||
rows = _logs([
|
||||
("q1", 2, 0.80), ("q2", 2, 0.75), ("q3", 2, 0.70),
|
||||
("q4", 2, 0.60), ("q5", 2, 0.50),
|
||||
])
|
||||
new = {"q1": 0.42, "q2": 0.38, "q3": 0.31, "q4": 0.22, "q5": 0.10}
|
||||
rescore = AsyncMock(side_effect=lambda u, q, p: new[q])
|
||||
|
||||
with patch.object(rm, "async_session", MagicMock(return_value=_session_with(rows))), \
|
||||
patch.object(rm, "floor_for", AsyncMock(return_value=0.72)), \
|
||||
patch.dict(rm._RESCORERS, {"prompt_rule": rescore}):
|
||||
out = await rm.migrate_floor(1, "prompt_rule")
|
||||
|
||||
assert out["old_admit_rate"] == 0.4 # 0.80 and 0.75 cleared 0.72
|
||||
assert out["proposed_floor"] == 0.38 # admits 0.42 and 0.38 — also two
|
||||
assert out["migrated"] is False # dry run by default
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_dry_run_writes_nothing():
|
||||
rows = _logs([("q1", None, 0.9), ("q2", None, 0.1)])
|
||||
with patch.object(rm, "async_session", MagicMock(return_value=_session_with(rows))), \
|
||||
patch.object(rm, "floor_for", AsyncMock(return_value=0.5)), \
|
||||
patch.object(rm, "set_dial", AsyncMock()) as set_dial, \
|
||||
patch.dict(rm._RESCORERS, {"prompt_rule": AsyncMock(return_value=0.4)}):
|
||||
out = await rm.migrate_floor(1, "prompt_rule")
|
||||
set_dial.assert_not_called()
|
||||
assert "proposed_floor" in out
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_applying_writes_an_ordinary_tuning_event_with_the_arithmetic():
|
||||
"""A migrated floor is reviewable and revertible like any other change."""
|
||||
rows = _logs([("q1", None, 0.9), ("q2", None, 0.1)])
|
||||
with patch.object(rm, "async_session", MagicMock(return_value=_session_with(rows))), \
|
||||
patch.object(rm, "floor_for", AsyncMock(return_value=0.5)), \
|
||||
patch.object(rm, "set_dial", AsyncMock(return_value={})) as set_dial, \
|
||||
patch.dict(rm._RESCORERS, {"prompt_rule": AsyncMock(return_value=0.4)}):
|
||||
out = await rm.migrate_floor(1, "prompt_rule", apply=True)
|
||||
|
||||
assert out["migrated"] is True
|
||||
kwargs = set_dial.call_args.kwargs
|
||||
reason = kwargs["reason"]
|
||||
# The reason has to carry the working, not just the verdict — it is what the
|
||||
# operator reads to decide whether to keep the number.
|
||||
assert "0.5" in reason and "sampled" in reason
|
||||
assert kwargs["actor"] == "model"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_logged_calls_refuses_rather_than_inventing_a_distribution():
|
||||
with patch.object(rm, "async_session", MagicMock(return_value=_session_with([]))), \
|
||||
patch.object(rm, "floor_for", AsyncMock(return_value=0.5)), \
|
||||
patch.object(rm, "set_dial", AsyncMock()) as set_dial:
|
||||
out = await rm.migrate_floor(1, "prompt_rule", apply=True)
|
||||
assert out["migrated"] is False
|
||||
assert "no logged calls" in out["why"]
|
||||
set_dial.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_corpus_that_rescores_to_nothing_refuses():
|
||||
"""The mid-backfill case: every bar would otherwise be set off no data."""
|
||||
rows = _logs([("q1", None, 0.9), ("q2", None, 0.8)])
|
||||
with patch.object(rm, "async_session", MagicMock(return_value=_session_with(rows))), \
|
||||
patch.object(rm, "floor_for", AsyncMock(return_value=0.5)), \
|
||||
patch.object(rm, "set_dial", AsyncMock()) as set_dial, \
|
||||
patch.dict(rm._RESCORERS, {"prompt_rule": AsyncMock(return_value=None)}):
|
||||
out = await rm.migrate_floor(1, "prompt_rule", apply=True)
|
||||
assert out["migrated"] is False
|
||||
assert "not embedded" in out["why"]
|
||||
set_dial.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_unknown_surface_is_refused():
|
||||
with pytest.raises(ValueError):
|
||||
await rm.migrate_floor(1, "promptrule")
|
||||
|
||||
|
||||
def test_the_tool_says_it_is_a_starting_point_and_defaults_to_a_dry_run():
|
||||
doc = tool_doc("scribe.mcp.tools.retrieval_tuning", "migrate_retrieval_floor")
|
||||
assert "DRY RUN BY DEFAULT" in doc
|
||||
# Percentile-preserving carries the old floor's wrongness forward faithfully.
|
||||
# A reader who misses that will treat a migrated number as a measured one.
|
||||
assert "STARTING POINT" in doc.upper()
|
||||
assert "stale" in doc
|
||||
@@ -183,17 +183,16 @@ def test_the_tool_teaches_reading_the_records_not_the_percentile():
|
||||
assert "69" in doc
|
||||
|
||||
|
||||
def test_all_three_tools_are_registered():
|
||||
def test_every_tool_in_the_module_is_registered():
|
||||
from scribe.mcp.tools import retrieval_tuning as tool
|
||||
|
||||
names = []
|
||||
from tests.helpers import FakeMCP
|
||||
|
||||
class _MCP:
|
||||
def tool(self, name):
|
||||
names.append(name)
|
||||
return lambda fn: fn
|
||||
|
||||
tool.register(_MCP())
|
||||
assert names == [
|
||||
"retrieval_surfaces", "tune_retrieval", "retrieval_tuning_history",
|
||||
mcp = FakeMCP()
|
||||
tool.register(mcp)
|
||||
# Order is the module's, and asserted rather than sorted: an unregistered
|
||||
# tool is invisible to every caller, so the list is worth reading literally.
|
||||
assert mcp.names == [
|
||||
"retrieval_surfaces", "migrate_retrieval_floor",
|
||||
"tune_retrieval", "retrieval_tuning_history",
|
||||
]
|
||||
|
||||
@@ -23,7 +23,7 @@ def test_backup_version_is_current():
|
||||
|
||||
(Named for the number it asserted until v10, which is exactly the drift a
|
||||
name-carrying-a-value invites; it now says what it checks.)"""
|
||||
assert backup.BACKUP_VERSION == 16
|
||||
assert backup.BACKUP_VERSION == 17
|
||||
|
||||
|
||||
def _exportable_note(**over):
|
||||
|
||||
@@ -0,0 +1,243 @@
|
||||
"""The wide net: many candidates, no bar, and a cost that stays bounded (#4103).
|
||||
|
||||
WHY THIS EXISTS
|
||||
|
||||
Milestone 416 step 5 moves "do not exclude potentially important data" off the
|
||||
push, where fifty candidates would be a wall nobody reads, and onto a pull,
|
||||
where they arrive only when a session asks for them.
|
||||
|
||||
Three things have to hold, and each has a way of quietly failing:
|
||||
|
||||
1. **No bar.** The moment this tool serves is the one where the caller does
|
||||
not trust a threshold to decide for them. A default floor creeping in
|
||||
here would turn the wide net into the narrow one and nothing would look
|
||||
wrong — the results would simply be fewer.
|
||||
2. **A bounded cost.** The step was filed claiming fifty "costs nothing".
|
||||
`_rule_hint_line` had already measured ~143 tokens for a line with its
|
||||
trigger rendered, so fifty is ~7,000. The graduated shape (#3851) is what
|
||||
keeps this affordable, and a change that renders every trigger in full
|
||||
would pass every other test here.
|
||||
3. **A pull, logged as one.** The push arms' near-miss distributions are the
|
||||
evidence #4121 argues from. A pull mixed into them moves those numbers.
|
||||
"""
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from scribe.mcp.tools import wide_net
|
||||
from tests.helpers import FakeMCP, fake_rule, tool_doc
|
||||
|
||||
|
||||
def _hits(n=3, trigger="Running git push", **over):
|
||||
"""n (score, rule) pairs, descending, the shape the service returns."""
|
||||
return [
|
||||
(0.9 - i * 0.01,
|
||||
fake_rule(id=i + 1, title=f"rule {i + 1}", when_to_apply=trigger, **over))
|
||||
for i in range(n)
|
||||
]
|
||||
|
||||
|
||||
def _patched(hits, report=None):
|
||||
"""Patch the search and the telemetry sink; hand back the search mock."""
|
||||
async def _search(*a, **kw):
|
||||
if report is not None and "report" in kw and kw["report"] is not None:
|
||||
kw["report"].update(report)
|
||||
return hits
|
||||
return patch.object(wide_net, "semantic_search_rules", AsyncMock(side_effect=_search))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_bar_reaches_the_search():
|
||||
"""THE POINT OF THE TOOL. A floor here would narrow the net silently."""
|
||||
with _patched(_hits()) as search, \
|
||||
patch.object(wide_net, "record_retrieval"), \
|
||||
patch.object(wide_net, "current_user_id", lambda: 1):
|
||||
await wide_net.what_might_apply("push to dev")
|
||||
|
||||
assert search.await_args.kwargs["threshold"] == 0.0
|
||||
assert wide_net.THRESHOLD == 0.0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("asked, expected", [(999, 50), (0, 1), (-5, 1), (25, 25)])
|
||||
async def test_the_limit_is_clamped_to_the_cap(asked, expected):
|
||||
with _patched(_hits()) as search, \
|
||||
patch.object(wide_net, "record_retrieval"), \
|
||||
patch.object(wide_net, "current_user_id", lambda: 1):
|
||||
await wide_net.what_might_apply("q", limit=asked)
|
||||
|
||||
assert search.await_args.kwargs["limit"] == expected
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_head_carries_its_trigger_whole_and_the_tail_is_cut():
|
||||
"""The graduated shape (#3851), which is what makes fifty affordable.
|
||||
|
||||
A regression that rendered every trigger in full would satisfy every other
|
||||
assertion in this file, so the cut is pinned on both sides: the head is
|
||||
NOT marked truncated and the tail IS.
|
||||
"""
|
||||
long_trigger = " ".join(["running a git command before pushing anything"] * 12)
|
||||
with _patched(_hits(6, trigger=long_trigger)), \
|
||||
patch.object(wide_net, "record_retrieval"), \
|
||||
patch.object(wide_net, "current_user_id", lambda: 1):
|
||||
out = await wide_net.what_might_apply("q", detail=2)
|
||||
|
||||
head, tail = out["candidates"][:2], out["candidates"][2:]
|
||||
assert all(c["truncated"] is False for c in head)
|
||||
assert all(c["when_to_apply"] == " ".join(long_trigger.split()) for c in head)
|
||||
assert all(c["truncated"] is True for c in tail)
|
||||
assert all(len(c["when_to_apply"]) <= wide_net._TEASER_CHARS + 1 for c in tail)
|
||||
assert out["detailed"] == 2
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_short_trigger_is_never_marked_truncated():
|
||||
"""`truncated` is a claim about this row, not about its rank. A tail row
|
||||
whose trigger already fits must not claim a cut that did not happen."""
|
||||
with _patched(_hits(4, trigger="Running git push")), \
|
||||
patch.object(wide_net, "record_retrieval"), \
|
||||
patch.object(wide_net, "current_user_id", lambda: 1):
|
||||
out = await wide_net.what_might_apply("q", detail=1)
|
||||
|
||||
assert all(c["truncated"] is False for c in out["candidates"])
|
||||
|
||||
|
||||
def test_the_cut_breaks_on_a_word_and_says_it_was_cut():
|
||||
"""#4036's lesson, borrowed: a raw slice ends mid-word and reads as the
|
||||
whole thing."""
|
||||
text, cut = wide_net._teaser("alpha beta gamma delta " * 40)
|
||||
assert cut is True
|
||||
assert text.endswith("…")
|
||||
# Broke on a word, so no partial token sits before the marker.
|
||||
assert not text.removesuffix("…").rstrip().endswith(("alph", "bet", "gam"))
|
||||
|
||||
|
||||
def test_one_unbroken_word_still_yields_text_rather_than_a_bare_marker():
|
||||
"""`textwrap.shorten` returns just "…" here, which would render a teaser
|
||||
carrying no information at all."""
|
||||
text, cut = wide_net._teaser("x" * 500)
|
||||
assert cut is True and text != "…"
|
||||
assert len(text) == wide_net._TEASER_CHARS
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_every_row_says_its_force_and_its_scope():
|
||||
"""`kind` is never inferred — a rule must be followed, a preference guides
|
||||
— and `scope` says how far a reader should generalise from it."""
|
||||
hits = [
|
||||
(0.8, fake_rule(id=1, kind="rule", project_id=None)),
|
||||
(0.7, fake_rule(id=2, kind="preference", project_id=44)),
|
||||
]
|
||||
with _patched(hits), patch.object(wide_net, "record_retrieval"), \
|
||||
patch.object(wide_net, "current_user_id", lambda: 1):
|
||||
out = await wide_net.what_might_apply("q")
|
||||
|
||||
assert [c["kind"] for c in out["candidates"]] == ["rule", "preference"]
|
||||
assert [c["scope"] for c in out["candidates"]] == ["global", "project"]
|
||||
|
||||
|
||||
# ── telemetry: a pull, and never mistaken for a push ────────────────────────
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_call_is_logged_under_its_own_pull_source():
|
||||
with _patched(_hits()), \
|
||||
patch.object(wide_net, "record_retrieval") as rec, \
|
||||
patch.object(wide_net, "current_user_id", lambda: 1):
|
||||
await wide_net.what_might_apply("push to dev")
|
||||
|
||||
kw = rec.call_args.kwargs
|
||||
assert kw["source"] == wide_net.SOURCE == "wide_net"
|
||||
assert kw["threshold"] == 0.0
|
||||
|
||||
|
||||
def test_the_wide_net_is_not_one_of_the_tunable_push_surfaces():
|
||||
"""THE GUARD that keeps #4121's evidence clean (rule 167).
|
||||
|
||||
The registry holds the PUSH arms — the ones with a floor and a budget the
|
||||
model tunes. This source must not appear there: a pull folded into those
|
||||
rows would move the near-miss distributions that step is arguing from, and
|
||||
would offer a floor to tune on a tool whose whole point is not having one.
|
||||
"""
|
||||
from scribe.services.retrieval_surfaces import SURFACES
|
||||
|
||||
assert wide_net.SOURCE not in SURFACES
|
||||
|
||||
|
||||
def test_the_wide_net_is_not_ambient_either():
|
||||
"""Ambient means a delivery nobody chose. This one is chosen by definition
|
||||
— somebody called the tool — so counting it as ambient would make a
|
||||
deliberate ask read as a bulk hand-over."""
|
||||
from scribe.services.note_usage import AMBIENT_SOURCES
|
||||
|
||||
assert wide_net.SOURCE not in AMBIENT_SOURCES
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_search_that_never_ran_is_not_reported_as_a_decline():
|
||||
"""#3765: an empty query, a dead embedder and a failed query all return
|
||||
nothing, and none of them is a ranker declining."""
|
||||
with _patched([], report={"searched": False, "best_available_score": None}), \
|
||||
patch.object(wide_net, "record_retrieval") as rec, \
|
||||
patch.object(wide_net, "current_user_id", lambda: 1):
|
||||
out = await wide_net.what_might_apply("")
|
||||
|
||||
assert rec.call_args.kwargs["searched"] is False
|
||||
assert out["searched"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_what_the_bar_turned_away_is_carried_through():
|
||||
"""There is no bar here, but `best_available` still answers "did the corpus
|
||||
have anything at all" for a call that came back empty (#3670)."""
|
||||
with _patched([], report={"searched": True, "best_available_score": 0.31,
|
||||
"best_available_id": 7}), \
|
||||
patch.object(wide_net, "record_retrieval") as rec, \
|
||||
patch.object(wide_net, "current_user_id", lambda: 1):
|
||||
await wide_net.what_might_apply("q")
|
||||
|
||||
assert rec.call_args.kwargs["best_available"] == 0.31
|
||||
assert rec.call_args.kwargs["best_available_id"] == 7
|
||||
|
||||
|
||||
# ── the contract a session actually reads ───────────────────────────────────
|
||||
|
||||
def test_the_docstring_says_when_to_reach_for_it():
|
||||
"""The step's done-when, and the load-bearing half of this tool.
|
||||
|
||||
A wide net nobody knows to call is worth nothing, so the docstring has to
|
||||
name the MOMENT, not just the parameters — including the one that prompted
|
||||
it, where a session hands work back rather than finishing it.
|
||||
"""
|
||||
doc = tool_doc("scribe.mcp.tools.wide_net", "what_might_apply").lower()
|
||||
|
||||
assert "consequential" in doc
|
||||
assert "handing work back" in doc
|
||||
# Says the tail is expected to be noise — otherwise the first caller reads
|
||||
# a low-scoring list as the tool being broken.
|
||||
assert "noise" in doc
|
||||
# And names its own limit: a pull only helps if somebody asks.
|
||||
assert "only helps if you ask" in doc
|
||||
|
||||
|
||||
def test_it_is_registered_and_readable_with_a_read_key():
|
||||
from scribe.mcp.server import _READ_ONLY_TOOLS
|
||||
|
||||
mcp = FakeMCP()
|
||||
wide_net.register(mcp)
|
||||
assert mcp.names == ["what_might_apply"]
|
||||
assert "what_might_apply" in _READ_ONLY_TOOLS
|
||||
|
||||
|
||||
def test_the_instruction_surfaces_point_at_it():
|
||||
"""Rule 119: the instruction surfaces ARE the specification for product
|
||||
behaviour, so a tool the reflex never learns about is not shipped."""
|
||||
import pathlib
|
||||
|
||||
from scribe.mcp import server
|
||||
|
||||
assert "what_might_apply" in server._INSTRUCTIONS
|
||||
|
||||
skill = (pathlib.Path(__file__).resolve().parents[1]
|
||||
/ "plugin" / "skills" / "using-scribe" / "SKILL.md").read_text()
|
||||
assert "what_might_apply" in skill
|
||||
Reference in New Issue
Block a user