Skip to content

feat(retrieval): explicit pins — a working set that always enters the pack - #666

Merged
plind-junior merged 5 commits into
vouchdev:testfrom
minion1227:feat/explicit-pins
Jul 30, 2026
Merged

feat(retrieval): explicit pins — a working set that always enters the pack#666
plind-junior merged 5 commits into
vouchdev:testfrom
minion1227:feat/explicit-pins

Conversation

@minion1227

Copy link
Copy Markdown
Contributor

Closes #615

when you are deep in one task there are three or four artifacts that should be in front of the agent every turn — the spec, the decision that constrains the design, the claim saying why the obvious approach was already rejected. whether they appear is up to the ranker, and a ranker optimising for the query drops them the moment the conversation moves.

$ vouch pin the-spec-says-tokens-rotate-every-24-hours --note "the constraint"
pinned claim/the-spec-says-tokens-rotate-every-24-hours (shared)

$ vouch pins list
shared  claim/the-spec-says-tokens-rotate-every-24-hours  — the constraint

the pinned claim now leads every pack, including one built from a query it shares no term with — which is the whole point, and what test_pinned_artifact_leads_a_pack_it_would_not_have_entered asserts.

the two invariants i most want reviewed

a pin is not a gate bypass. it points at an artifact that is already approved, asserts nothing new, and creates nothing durable — so there is nothing for a reviewer to review. an unapproved artifact cannot be pinned at all because it does not resolve (test_pinning_an_unknown_artifact_is_refused). the issue says this too; i am repeating it because "always enters the context pack" is the kind of phrase that should attract scrutiny.

a pin is not a permission. lifecycle and viewer scope are re-checked on every build, not at pin time:

  • a pinned claim that is later superseded, archived or redacted stops being injected — parametrized over archive and supersede
  • a pinned page that is archived stops being injected
  • a pinned claim the viewer's scope hides is never injected

this is the "retracted content leaking into a read surface" bug class that has now been fixed on digest, salience, context, search, hot-memory, triage, verify — and twice on explain_ranking after i shipped it (#640, #651). so it is pinned by tests here from the start rather than after. a pin records what to prefer, never a right to see it.

budget

pins are capped at retrieval.pins.budget_share (default 0.3) of the pack budget, so they cannot starve retrieval. the tail is dropped at the cap rather than skipping a large pin to fit a later small one — pin order is the user's stated priority and silently reordering it would be worse than dropping the end. one pin always survives, even at a share of ~0, because a working set of zero is not what anyone asked for.

pins are also de-duplicated against what retrieval already found, so a pinned artifact that also ranks takes one slot, not two.

storage

.vouch/pins.yaml is committed — a team shares one working set and changes to it show up in review like everything else. --local writes .vouch/pins.local.yaml and backfills .gitignore using the same helper shape retrieval_events uses for telemetry. when an artifact is pinned in both, shared wins.

--expires 7d drops a pin automatically, and expiry is applied on read rather than by rewriting the file — building a context pack must never write. test_expired_pins_are_dropped_on_read_without_rewriting asserts the file is byte-identical afterwards.

no kb.* method — deliberately

the issue's open question is whether agents may pin, and proposes: agents suggest, humans confirm. i took that answer, so there is nothing registered on the agent surfaces and no four-site registration. if you would rather agents manage their own pins, kb.pin / kb.unpin is an additive follow-up rather than a rework — but i did not want to widen the write-ish surface on my own reading of an open question.

one thing i had to fix while wiring the cli: PinError was escaping as a raw traceback because _cli_errors catches an explicit tuple. it is registered there now, so it renders as a one-line Error: ... like every other domain error — test_cli_unknown_artifact_is_a_clean_error asserts no traceback.

tests

41 cases in tests/test_pins.py: the pack-ordering behaviour, both "not a permission" families (lifecycle × archive/supersede, archived page, viewer scope), budget share including the ~0 edge and the defensive config coercions, expiry, local vs shared precedence, re-pinning, malformed pin rows and unreadable files, deleted-artifact skips, and the full cli round trip in text and json.

verification

pytest tests/ -q --ignore=tests/embeddings   green
mypy src                                     Success: no issues found in 118 source files
ruff check src tests                         All checks passed!
diff-cover --fail-under 100                  100%, 236/236 changed lines in src/vouch

context.py gains 12 lines and no behaviour change when no pins are set — the pin file does not exist on a stock KB, so pinned_items returns empty and the pack is byte-identical.

… pack

when you are deep in one task there are three or four artifacts that should
be in front of the agent every turn — the spec, the decision that constrains
the design, the claim saying why the obvious approach was already rejected.
whether they appear is currently up to the ranker, and a ranker optimising
for the query drops them the moment the conversation moves. hot_memory and
salience are the implicit version of this and decay exactly when a long task
needs them not to.

pinned claims and pages now lead the pack. they are capped at
retrieval.pins.budget_share (default 0.3) so pins can never starve
retrieval, and de-duplicated against what retrieval already found so a
pinned artifact that also ranked does not take two slots. pin order is the
user's stated priority, so the tail is dropped at the budget rather than
silently reordered.

a pin is not a gate bypass. it points at an artifact that is already
approved, asserts nothing new, and creates nothing durable — there is
nothing for a reviewer to review, and an unapproved artifact cannot be
pinned because it does not resolve.

a pin is also not a permission. lifecycle and viewer scope are re-checked on
every build: a pinned claim that is later superseded, archived or redacted
stops being injected, as does a pinned page that is archived or one the
scope filter hides. the pin records what to prefer, never a right to see it.
this is the bug class that has been fixed a dozen times on other read
surfaces, so it is pinned by tests here from the start rather than after.

shared pins live in committed .vouch/pins.yaml so a team shares one working
set and changes show up in review; --local keeps a personal set in
gitignored .vouch/pins.local.yaml, using the same .gitignore backfill
retrieval telemetry uses. --expires drops a pin automatically, applied on
read rather than by rewriting the file, because building a context pack must
not write.

no kb.* method: the issue's own open question proposes that pinning is a
human act and agents may only suggest, so there is nothing to register on
the agent surfaces.

Closes vouchdev#615
@minion1227
minion1227 requested a review from plind-junior as a code owner July 30, 2026 18:35
@github-actions github-actions Bot added docs documentation, specs, examples, and repo guidance cli command line interface retrieval context, search, synthesis, and evaluation tests tests and fixtures size: L 500-999 changed non-doc lines labels Jul 30, 2026

@plind-junior plind-junior left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implements #615 faithfully — vouch pin / pins list / unpin, pins lead every pack within retrieval.pins.budget_share, committed shared set plus --local, expiry applied on read, and the issue's open question (agents pin?) resolved conservatively with no kb.* surface. the two invariants you asked to have reviewed (not a gate bypass, not a permission) are genuinely enforced in pinned_items and pinned by tests: lifecycle and scope are re-checked inside the loop before budget is consumed, the one-pin floor works, PinError renders cleanly, and the expiry-on-read byte-identical-file assertion is exactly the right test.

one blocking bug in --expires parsing and one non-blocking ordering nit — both anchored inline on the lines in question.

requesting changes for the --expires <iso-date> inversion: it silently no-ops a documented input. the fix and its regression test are small; everything else is ready.

Comment thread src/vouch/cli.py Outdated
Comment thread src/vouch/context.py Outdated
parse_since returns iso input unchanged and only counts durations
backwards, so mirroring its result around now was right for 7d and wrong
for a date: --expires 2026-08-15 computed now + (now - 2026-08-15), a
timestamp a month in the past. load_pins drops expired pins on read, so
the pin vanished the moment it was written, with no error.

iso input is now parsed directly and only durations are mirrored. a spec
resolving to no bound (all, empty) is rejected rather than silently
meaning never, which is already what omitting the flag does.

also de-duplicate retrieval against pins on near-duplicate text, not just
exact (type, id). _dedupe_near_duplicates runs before pins are injected,
so it never compares a retrieved item to a pin, and the same knowledge
stored under a second id could take a second slot. the comparison runs at
the injection site rather than by moving that pass below it: the pass
keeps the highest-scored member of a cluster, and pages_first multiplies
a page's score past a pin's flat 1.0, so moving it would let retrieval
evict a pin.
@minion1227

Copy link
Copy Markdown
Contributor Author

both fixed, and the second one differently than you suggested — the suggested move would have broken the invariant it was protecting.

the --expires inversion. fixed as described: iso input is parsed directly and only durations get mirrored. worth recording how bad it was, since it is worse than "silently no-ops":

'7d'          parse_since=2026-07-23 19:18  -> expires_at=2026-08-06 19:18  ok
'2026-08-15'  parse_since=2026-08-15 00:00  -> expires_at=2026-07-15 14:36  ALREADY EXPIRED

load_pins drops expired pins on read, so the pin did not merely arrive with a bad date — it vanished the moment it was written. vouch pins list came back empty and nothing errored. the regression test asserts the pin is present in load_pins as well as future-dated, which is the assertion that actually fails on the old code (the old behaviour raises IndexError there).

i also took the non-blocking nit you folded into the same raise: all and "" now error instead of silently meaning never. parametrised over both, plus a garbage spec — MetricsError subclasses ValueError, so _cli_errors already renders it as a clean Error: line and nothing extra was needed there.

the near-duplicate ordering — i did not move the pass. _dedupe_near_duplicates does not keep the first occurrence; it iterates in descending score order and keeps the highest-scored member of a cluster (context.py:738, survivors returned in original order). and _maybe_pages_first multiplies a page's score by boost, default 1.25 and configurable to anything positive, while pinned_items gives every pin a flat score=1.0. so a retrieved page that near-duplicates a pin can score above it, and moving the pin injection above the pass would let retrieval evict the pin — exactly the guarantee the feature exists to make.

so i took the fallback you named: the pass stays where it is and pins de-duplicate against retrieval explicitly at the injection site, where the pin always wins. the heuristic (0.85 jaccard over the first 40 tokens) is now two named constants shared by both call sites so they cannot drift to different notions of "same".

test_near_duplicate_eviction_keeps_the_pin_not_the_higher_score pins that with boost: 5.0 — it fails on the moved-pass version. one honest note: test_a_pin_evicts_its_near_duplicate_from_retrieval passes both before and after, because at those scores the near-dup pass already drops the twin during retrieval; it documents the invariant but the page/boost test is the one guarding the change.

mypy src clean at 119 files, ruff clean, tests/test_pins.py and tests/test_context.py green (65 cases). also merged test in to clear BEHIND.

@github-actions github-actions Bot added size: XL 1000 or more changed non-doc lines and removed size: L 500-999 changed non-doc lines labels Jul 30, 2026
@plind-junior
plind-junior enabled auto-merge July 30, 2026 19:30

@plind-junior plind-junior left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good!

@github-actions
github-actions Bot disabled auto-merge July 30, 2026 19:41
@plind-junior
plind-junior merged commit aafd096 into vouchdev:test Jul 30, 2026
17 checks passed
@minion1227
minion1227 deleted the feat/explicit-pins branch July 31, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli command line interface docs documentation, specs, examples, and repo guidance retrieval context, search, synthesis, and evaluation size: XL 1000 or more changed non-doc lines tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(retrieval): explicit pins — a working set that always enters the pack

2 participants