feat(sdk): adopt PostHog's intent description as the default (0.3.1) - #70
Merged
Conversation
Our default told the model to use 15-25 words, third person, AND gave it a worked example — so the model copied the example's shape. Of 59 intents captured on Preisvergleich Prod in the 30 days to 2026-08-07, the ChatGPT ones averaged 122 characters and nearly every one opened "Comparing X to help the user Y": "Comparing German private current accounts for a €2,500 monthly income to identify suitable fees, conditions, and account options." "Comparing one-year fixed-deposit offers to help the user evaluate returns on a €10,000 investment." Informative, and interchangeable — which defeats the point of a free-text field. PostHog ships one unconstrained question instead, so that is now the default verbatim: "Why are you calling this tool? Briefly describe the user's goal." It also costs ~11 words of every tool's schema instead of ~40. Deliberately dropped: the old "Never include credentials or personal data" clause. It was not doing the job — the samples above carry income and investment figures with that sentence in place — and a description the model may reinterpret is the wrong layer for a privacy guarantee. The guarantees that actually hold are unconditional and downstream: stripPii() over intent_signals in buildToolCallEvent, and the 500-character clamp. The docs now say so explicitly rather than implying the description protects anything. Two things found while bumping the version, both fixed here: - SDK_VERSION is hardcoded in TWO places (server entrypoint and the React bundle, which cannot import the server one) but only the server copy was checked against package.json. A release could have shipped widget events stamped 0.3.0 while everything else said 0.3.1, silently. Both are now covered; the new test fails with "expected '0.3.0' to be '0.3.1'" when the React constant is left behind. - Three React tests asserted the literal "0.3.0" rather than the constant, so they broke on every version bump for no reason. They now compare against SDK_VERSION, and the value itself is guarded by the package.json checks. 300/300 SDK tests pass.
Coverage Report for sdk
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…lamping
Follow-up to the review of this PR. The wording change was fine; the
justification around it was not, and one real ordering bug came with it.
The docs callout I added promised "intent_signals is PII-stripped" as though
that settled the matter — while citing, two sentences earlier, an income figure
the stripper provably cannot catch. stripPii matches exactly four shapes
(email, Luhn-valid card, US SSN, US-format phone) and the ingest layer adds
ITIN and US street addresses; pii.ts calls itself "best-effort" and
"defense-in-depth" for good reason. A customer citing that sentence in a DPIA
would have been relying on something the code does not do. The docs now name
the four shapes, list what is stored verbatim (income, names, employers,
non-US formats, pasted credentials), and say plainly that `intent: false` is
the answer for regulated data.
The config comment had the same problem in reverse: it justified deleting the
"never include personal data" clause by asserting the downstream layer was the
real guarantee. It is not, and the comment's own evidence proves it. It now
says the trade was a weak control for none, and no longer forbids re-adding a
sentence — it just refuses to call one a control.
Ordering bug, found by the same review and fixed here: normalizeIntent clamped
to 500 and left redaction to buildToolCallEvent, so
- PII straddling the 500-char boundary was truncated into a fragment the
patterns no longer match — "alice@corp." with the TLD sliced off shipped
looking scrubbed while still being linkable; and
- redaction tokens are longer than what they replace ("a@b.co" 6 chars ->
"[EMAIL_REDACTED]" 16), so the stored value could exceed the 500 characters
the docs promise.
Redaction now runs before the clamp, behind a generous safety ceiling that
bounds the regex work on unbounded model output. The old clamp test used
"x".repeat(), which redaction never touches, so it could not have caught
either half.
Also from the review:
- SDK_VERSION is now a single core/version.ts re-exported by both entrypoints,
rather than two literals plus a test to police them. The React bundle could
always import core/ (react/hook.ts already imports core/pii.js), so the
duplication was never structural. The test now guards the arrangement — it
fails if a literal reappears — instead of the value.
- intent_signals had no redaction test at all; deleting the stripPii call left
the suite green. It now has one.
- The docs quote the default description verbatim, which reintroduced the
"edit both" drift this PR removed elsewhere. A test pins the page to
DEFAULT_INTENT_DESCRIPTION.
- The docs example was itself a third-person gerund sentence — the exact
converged shape the surrounding paragraph criticises. Replaced, and labelled
as not a template.
All four new guards verified to fail when the behaviour they cover is broken.
305/305 SDK tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes PostHog's wording the SDK default, so both instrumented apps pick it up from an SDK bump rather than each hardcoding a string.
Why
The old default prescribed a word count, third person and a worked example — so the model copied the example's shape. Of 59 intents captured on Preisvergleich Prod in the 30 days to 2026-08-07, the ChatGPT ones averaged 122 characters and nearly every one opened the same way:
Informative, and interchangeable. The new default is one open question — and costs ~11 words of every tool's schema instead of ~40.
The privacy clause is gone on purpose
The old text asked the model never to include personal data. Look at the samples above — it included stated monthly income and investment amounts anyway. A description is an instruction the model can reinterpret; it was never a control. The controls that do hold are unconditional and downstream:
stripPii()overintent_signalsinbuildToolCallEvent, and the 500-character clamp. The docs now say that outright, with a callout, instead of leaving readers to assume the description protects them.If you'd rather keep a privacy sentence anyway, say so and I'll put one back — but I'd rather it not read as a guarantee.
Two bugs found while bumping the version
Both fixed here, and worth knowing about independently of the wording:
SDK_VERSIONis hardcoded in two places — the server entrypoint and the React bundle, which can't import the server one — but only the server copy was checked againstpackage.json. A release could have shipped widget events stamped0.3.0while everything else said0.3.1, with nothing to catch it. Both are covered now. Verified with teeth: leaving the React constant behind fails withexpected '0.3.0' to be '0.3.1'."0.3.0"rather than the constant, so they broke on every version bump for no reason. They compare againstSDK_VERSIONnow, with the value itself guarded by thepackage.jsonchecks above.Verified
300/300 SDK tests pass, typecheck and lint clean. Confirmed against a real
McpServerthat the advertisedtools/listschema carriescontext.description = "Why are you calling this tool? Briefly describe the user's goal.", and that nocontextis advertised when intent is off.What follows this
This is the first of three steps and the only one that needs your review:
@yavio/sdk0.3.1 — tagsdk-v0.3.1, OIDC publish; manual and yoursOne expectation to set: Shipal will take the new wording but won't produce more intents until its capture bug is fixed — it's at 3.2% coverage versus Preisvergleich's 100%. The signal for whether this wording is better will come from Preisvergleich.