[pull] main from danny-avila:main - #152
Merged
Merged
Conversation
…ion, Shared Marker (#353) * 🏷️ refactor: Drop the Tense Verb Map, Trim the Intent Description, Export the Marker - Removed OUTCOME_VERB_MAP and the mechanical present-progressive→past rewrite. A closed English verb list is wrong three ways at once: it never fires for the non-English labels this feature expects (the model answers in the user's language), it fires for some sibling calls and not others inside one group — "Searched…" beside "Recording…", which a real run produced — and it enumerates a vocabulary in a feature whose premise is a free-form sentence. applyOutcome now returns the intent unchanged when the tool authored nothing; completion is a UI state (the shimmer stopping, the icon settling), which is language-neutral and always consistent. Tools that want past tense still say so explicitly through outcome / outcome_patch. - Trimmed INTENT_DESCRIPTION 502 → 289 chars (~126 → ~72 tokens). It is repeated on every opted-in schema on every request, so roughly 800 tokens per request come back across a full coding bundle. Every load-bearing clause is kept: first-position placement, the one-sentence present-progressive form, who reads it, and the sibling rule the headline case depends on. - Exported INTENT_LABEL_MARKER. The description's opening words are a discriminator that host applications reimplement, and a duplicated string literal that drifts makes the host silently stop recognizing SDK-native labels — failing OPEN, with no error. One definition now. - Added withoutIntent(): the opt-out embedders had no lever for. Native schemas apply withIntent at module scope, so a consumer that renders no status label previously had no way to avoid the tokens. Marker-guarded, so a tool's own business `intent` parameter is never removed. * 📝 docs: Correct resolveToolOutcome's Contract After the Tense Transform Removal The JSDoc still told embedders the mechanical transform was 'left to the host' and that unmatched patches must not fall through to it — a host mirroring that contract would reimplement exactly the locale- and vocabulary-dependent inconsistency this branch removes. States the real contract now: a bare intent is displayed unchanged, hosts must not rewrite it, and an unmatched patch on a failed call resolves to undefined. * 📝 docs: Purge the Tense-Transform Contract from the Public Types and Search Tool ProcessedToolCall.outcome — the EXPORTED contract hosts implement against — still told them to apply the mechanical intent transform when outcome is absent. That is the one place a stale instruction does the most damage: a host following it faithfully reintroduces the locale split and inconsistent sibling labels this branch removes. It now states the real rule: display the intent unchanged, never rewrite its tense, and why. My previous sweep only covered intentArg.ts and types/tools.ts, so a repo-wide pass found two more: resolveSearchOutcome's JSDoc described a zero-result search as deferring to 'the host's mechanical past-tense transform', and two test names still described behavior that no longer exists. All corrected; a full grep now returns only the passages that explain the transform's deliberate absence. * 🧯 fix: Make withoutIntent Usable on the Schemas It Exists For Both findings land on the opt-out helper added earlier in this branch, and together they meant the advertised API did not work on its primary target. - Accepts readonly schemas. The SDK's native schemas are declared `as const`, so their `required` is a readonly tuple; a mutable `string[]` parameter rejected them with TS2345, forcing embedders to cast to call the helper documented as their opt-out. The parameter is now `IntentStrippableSchema`, widening `required` to `readonly string[]`. A test calls `withoutIntent(ReadFileToolSchema)` directly — it is a compile-time assertion, not just a runtime one. - Prunes `required` alongside the property. A schema listing `intent` as required — which strict-mode normalization produces, since OpenAI strict function schemas require every property in `required` — was left naming a property it no longer declared. That is invalid JSON Schema, so the provider rejects the request instead of the embedder quietly opting out. `required` is filtered, and dropped entirely when `intent` was its only entry.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )