From f5a710c5857a59dde50ab60c760c8aed4615be05 Mon Sep 17 00:00:00 2001 From: Yiming Su Date: Mon, 22 Jun 2026 16:16:15 -0400 Subject: [PATCH] docs(adk): fix skill gaps surfaced by the 06-20 benchmark sweep (#52) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(adk): fix skill gaps surfaced by the 06-20 benchmark sweep Each change was verified against agent-lack source (runtime/CLI/bundler) before editing. Skills-only; no runtime/CLI changes. - ADK-702: custom events nest authored data at event.payload.payload, not event.payload (conversations.md, patterns-mistakes.md) - ADK-703: route natural language to execute()/adk.zai.extract instead of hand-rolled keyword/regex parsers (conversations.md, patterns-mistakes.md) - ADK-704: single-quote `adk chat --single` messages; $ expands in double quotes and silently mangles input (cli.md, adk-test.md) - ADK-705: test pushed chat:custom events with an eval event turn + adk evals, not adk chat --single or curl (adk-test.md, debug-workflow.md, conversations.md) - ADK-708/707: ship bundled data via static JSON import; assets.get() returns a URL only, never file bytes (patterns-mistakes.md, assets.md) - eval event turns take { payload } only (no type field); pushed events arrive as chat:custom (adk-evals SKILL.md, eval-format.md, test-patterns.md) Co-Authored-By: Claude Opus 4.8 (1M context) * chore: format .claude-plugin manifests with oxfmt Pre-existing format:check failures on dev (multi-line keywords arrays), unrelated to the skill doc changes — fixes the Code Quality check. Co-Authored-By: Claude Opus 4.8 (1M context) * docs(adk): address review — guard message access + link zai reference - patterns-mistakes.md: use message?.payload.text in the WRONG routing example so it doesn't model an unguarded-access crash on event turns - conversations.md: cross-link zai-agent-reference.md where adk.zai.extract is mentioned, so the API and its import are discoverable Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .claude-plugin/marketplace.json | 11 +------- .claude-plugin/plugin.json | 7 +---- commands/adk-test.md | 4 +-- .../adk-debugger/references/debug-workflow.md | 2 ++ skills/adk-evals/SKILL.md | 12 ++++---- skills/adk-evals/references/eval-format.md | 15 ++++++---- skills/adk-evals/references/test-patterns.md | 5 ++-- skills/adk/references/assets.md | 2 ++ skills/adk/references/cli.md | 10 ++++--- skills/adk/references/conversations.md | 10 +++++-- skills/adk/references/patterns-mistakes.md | 28 ++++++++++++++++++- 11 files changed, 66 insertions(+), 40 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 8d66558..af93374 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -17,16 +17,7 @@ "homepage": "https://github.com/botpress/skills", "repository": "https://github.com/botpress/skills", "license": "MIT", - "keywords": [ - "botpress", - "adk", - "agent", - "integration", - "ai", - "evals", - "debugging", - "frontend" - ], + "keywords": ["botpress", "adk", "agent", "integration", "ai", "evals", "debugging", "frontend"], "category": "development", "strict": false } diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 8477fd1..c1bf908 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -9,11 +9,6 @@ "homepage": "https://github.com/botpress/skills", "repository": "https://github.com/botpress/skills", "license": "MIT", - "keywords": [ - "botpress", - "adk", - "agent", - "ai" - ], + "keywords": ["botpress", "adk", "agent", "ai"], "category": "development" } diff --git a/commands/adk-test.md b/commands/adk-test.md index 6370fa5..eeebc27 100644 --- a/commands/adk-test.md +++ b/commands/adk-test.md @@ -14,10 +14,10 @@ If `$ARGUMENTS` is empty, list the user's primitives and ask which to test. 1. **Locate and identify type.** Glob `src/**/.ts`. The directory tells you the type (action / tool / workflow / conversation / table / trigger / knowledge base). 2. **Build an invocation appropriate for the type.** If the user provided input after the primitive name, use it as-is; otherwise generate a realistic probe from the primitive's schema: - - **Tool / conversation handler / agent step:** `adk chat --single "" --format json`. The probe should plausibly cause the LLM to call the tool / route to the handler. + - **Tool / conversation handler / agent step:** `adk chat --single '' --format json`. The probe should plausibly cause the LLM to call the tool / route to the handler. Single-quote the message — in double quotes the shell expands `$`, so a probe like `"I spent $5"` reaches the bot mangled. - **Action:** invoke through `adk run .adk/scratch/test-.ts` after writing a small disposable runner under `.adk/scratch/`, or via a chat probe that triggers a tool which calls the action. Prefer the runner when the action has a deterministic input shape. - **Workflow:** trigger via the documented entry point (chat probe, action call, or trigger event). - - **Trigger:** simulate the source event the trigger subscribes to. If the event source is external, explain the limitation and fall back to invoking the trigger handler directly via `adk run`. + - **Trigger:** a `Trigger` fires on an external source event that usually can't be produced locally — invoke its handler directly via `adk run` with a synthetic event. (A Conversation's pushed `chat:custom` event is different: test it with an eval `event` turn and `adk evals` — see `/adk-eval`. `adk chat --single` only sends user text and can't push one.) - **Table:** insert a sample row, run a representative query, then **delete the row** before reporting. Tag the test row with a recognizable marker (e.g., a `__test_` value in a string column) so the cleanup query is unambiguous. If the project is linked to a shared or production workspace, ask the user before inserting at all — offer to scope the test to a local dev table instead. - **Knowledge base:** run a search query against it. 3. **Run it and read traces.** Capture the response. Run `adk traces --format json` filtered to the most recent invocation if the response alone is not enough to judge correctness. diff --git a/skills/adk-debugger/references/debug-workflow.md b/skills/adk-debugger/references/debug-workflow.md index 53206a7..b6dccd0 100644 --- a/skills/adk-debugger/references/debug-workflow.md +++ b/skills/adk-debugger/references/debug-workflow.md @@ -51,6 +51,8 @@ adk chat --single "the message that triggers the bug" --format json **Why minimal?** A single message isolates the problem. Most often, multi-turn conversations add noise — the issue might be in turn 3 but caused by context from turn 1. That being said, some bugs are only reproducible by going through many turns. +**Pushed-event or proactive bug?** `adk chat --single` only sends user text — it can't push a `chat:custom` event. Reproduce with an eval `event` turn (`event: { payload: {...} }`) and run `adk evals`; don't curl the chat webhook. + **Save the conversation ID** from the JSON output — you'll need it for trace queries and follow up conversations. If the issue requires multi-turn context: diff --git a/skills/adk-evals/SKILL.md b/skills/adk-evals/SKILL.md index cdd58ba..611faa7 100644 --- a/skills/adk-evals/SKILL.md +++ b/skills/adk-evals/SKILL.md @@ -94,11 +94,11 @@ export default new Eval({ ### Turn types -| Turn | When to use | -| -------------------------- | ------------------------------------------------ | -| `user: 'message'` | Standard user message | -| `event: { type, payload }` | Non-message trigger (webhook, integration event) | -| `expectSilence: true` | Assert bot does NOT respond | +| Turn | When to use | +| --------------------- | ---------------------------------------------- | +| `user: 'message'` | Standard user message | +| `event: { payload }` | Push a custom event (arrives as `chat:custom`) | +| `expectSilence: true` | Assert bot does NOT respond | ### Assertion categories @@ -134,7 +134,7 @@ adk evals runs --latest -v # with full details ```typescript // CORRECT { user: 'hello', expectSilence: true } -{ event: { type: 'payment.failed' }, expectSilence: true } +{ event: { payload: { kind: 'payment.failed' } }, expectSilence: true } ``` ❌ **`expectSilence` alone is not a valid turn** diff --git a/skills/adk-evals/references/eval-format.md b/skills/adk-evals/references/eval-format.md index 6bdb5fe..fda071a 100644 --- a/skills/adk-evals/references/eval-format.md +++ b/skills/adk-evals/references/eval-format.md @@ -85,12 +85,11 @@ Each entry in `conversation` is one turn. A turn must have either `user` or `eve ### Event Trigger -Fire a non-message event (webhook, integration event) instead of a user message. +Push a custom event instead of a user message. The turn carries only a `payload`; the bot receives it as a `chat:custom` event and reads the data at `event.payload.payload`. ```typescript { event: { - type: 'checkout:order_placed', payload: { orderId: 'ORD-001', total: 49.99 }, }, assert: { @@ -108,7 +107,7 @@ Assert the bot does **not** respond. Add `expectSilence: true` to any turn. { user: 'Please ignore this.', expectSilence: true } // Silence after an event -{ event: { type: 'internal:ping' }, expectSilence: true } +{ event: { payload: { kind: 'ping' } }, expectSilence: true } ``` > **Note:** `expectSilence` is mutually exclusive with `assert.response`. Every turn must have `user` or `event` — `expectSilence` is a flag on top of that, not a standalone turn type. @@ -347,13 +346,19 @@ export default defineConfig({ ```typescript // WRONG — mutually exclusive -{ user: 'hello', event: { type: 'payment.failed' } } +{ user: 'hello', event: { payload: { amount: 50 } } } ``` ✅ **Correct — use separate turns** ```typescript -{ event: { type: 'payment.failed', payload: { amount: 50 } } } +{ + event: { + payload: { + amount: 50 + } + } +} ``` ## See Also diff --git a/skills/adk-evals/references/test-patterns.md b/skills/adk-evals/references/test-patterns.md index 7f8cb9c..c30f0d1 100644 --- a/skills/adk-evals/references/test-patterns.md +++ b/skills/adk-evals/references/test-patterns.md @@ -192,7 +192,6 @@ export default new Eval({ conversation: [ { event: { - type: 'payment:failed', payload: { amount: 99.99, currency: 'USD', customerId: 'cust-001' }, }, assert: { @@ -213,8 +212,8 @@ export default new Eval({ conversation: [ { - event: { type: 'internal:heartbeat' }, - expectSilence: true, // bot should not respond to internal events + event: { payload: { kind: 'heartbeat' } }, + expectSilence: true, // bot should not respond to heartbeat payloads }, ], }) diff --git a/skills/adk/references/assets.md b/skills/adk/references/assets.md index 0cf98b6..6d19374 100644 --- a/skills/adk/references/assets.md +++ b/skills/adk/references/assets.md @@ -67,6 +67,8 @@ console.log(logo.mime) // "image/png" console.log(logo.size) // bytes ``` +> `assets.get()` returns **metadata only** (URL, mime, size) — never the file's bytes. To ship a data table your code reads in-process (menu, catalog, rules), don't use assets — `import` the JSON statically instead. See [patterns-mistakes.md → Shipping a bundled data file](./patterns-mistakes.md#shipping-a-bundled-data-file). + ### List All Assets ```typescript diff --git a/skills/adk/references/cli.md b/skills/adk/references/cli.md index 01e1fa2..e9b93ca 100644 --- a/skills/adk/references/cli.md +++ b/skills/adk/references/cli.md @@ -362,12 +362,14 @@ adk chat [options] ```bash adk chat # interactive -adk chat --single "" # one-shot -adk chat --single "" --format json -adk chat --single "Run the full analysis" --timeout 30s -adk chat --single "Follow up" --conversation-id +adk chat --single '' # one-shot +adk chat --single '' --format json +adk chat --single 'Run the full analysis' --timeout 30s +adk chat --single 'Follow up' --conversation-id ``` +> **Single-quote the message.** In double quotes the shell expands `$`, so `"I spent $5"` reaches the bot as `I spent ` (and `$10`→`0`, `$84.50`→`4.50`). It looks like the bot can't parse the input, but it's shell mangling. Use single quotes: `adk chat --single 'I spent $5'`. + **Requires:** - `adk dev` running (also creates the `devId` on first run). Conversation continuation requires the dev server to be running — the user token is persisted automatically. diff --git a/skills/adk/references/conversations.md b/skills/adk/references/conversations.md index 660e294..89eb8f1 100644 --- a/skills/adk/references/conversations.md +++ b/skills/adk/references/conversations.md @@ -138,6 +138,8 @@ export const Chat = new Conversation({ See **[Messages](./messages.md)** for complete guide on all message types, metadata, and sending patterns. +> **Route natural language to `execute()` — don't hand-roll parsers.** Match exact strings only for explicit slash-commands (`/help`) or structured event `type`s. For anything users phrase in their own words, call `execute({ instructions })` and pull structured fields with `adk.zai.extract` (see [zai-agent-reference.md](./zai-agent-reference.md)); keyword/regex/`startsWith` matching silently misses real phrasings. + ## Custom & Proactive Events To push a message into a conversation from an external trigger (e.g. an announcement or status update), handle a custom event **inside the Conversation** — declare it in `events` and branch on `type === 'event'`. Don't build a separate `Trigger` + `client.createMessage`; an integration can't post messages as itself. @@ -148,9 +150,9 @@ export const Chat = new Conversation({ events: ['chat:custom'], async handler({ type, event, conversation }) { if (type === 'event') { - // The pushed data is in event.payload. On channel '*' the handler types - // event as `unknown`, so read its payload with a cast (or use a concrete channel). - const { orderId } = (event as { payload: { orderId: string } }).payload + // The pushed data is nested at event.payload.payload. On channel '*' the handler + // types event as `unknown`, so read it with a cast (or use a concrete channel). + const { orderId } = (event as { payload: { payload: { orderId: string } } }).payload.payload await conversation.send({ type: 'text', payload: { text: `Order ${orderId} shipped!` } }) return } @@ -159,6 +161,8 @@ export const Chat = new Conversation({ }) ``` +To test a pushed event without an external trigger, fire it from an eval `event` turn (`event: { payload: {...} }`) and run `adk evals` — `adk chat --single` only sends user text. See the `adk-evals` skill. + ## Conversation Instance Methods The `conversation` object provides methods for interacting with the current conversation. diff --git a/skills/adk/references/patterns-mistakes.md b/skills/adk/references/patterns-mistakes.md index 655b20a..1e8b1d4 100644 --- a/skills/adk/references/patterns-mistakes.md +++ b/skills/adk/references/patterns-mistakes.md @@ -37,6 +37,16 @@ const { myAction } = await import('./actions/myAction') import { myAction } from './actions/myAction' ``` +### Shipping a bundled data file + +To ship a data table your code reads at runtime (menu, catalog, pricing rules), **import the JSON statically** — the bundler inlines it into the build: + +```typescript +import menu from './data/menu.json' // ✅ inlined at build time +``` + +Don't reach for `fs.readFile` (the file isn't on disk at runtime → ENOENT), `import.meta`/`fileURLToPath` (undefined in the bundle → throws), or `assets.get()` (returns a URL, not the file's contents). All three pass `adk check` and `tsc` but crash only at runtime. + ### Handler Syntax #### Action Handlers @@ -80,12 +90,28 @@ An event pushed into a conversation (e.g. an order shipped, a status update rela // ❌ WRONG - pushed event is chat:custom, so this never fires events: ['claimUpdate'], -// ✅ CORRECT - branch on type === 'event', read event.payload +// ✅ CORRECT - branch on type === 'event', read event.payload.payload events: ['chat:custom'], ``` The camelCase rule applies only to events you define/emit, not to receiving. See [conversations.md → Custom & Proactive Events](./conversations.md#custom--proactive-events). +### Conversation Routing Mistakes + +#### Don't hand-roll natural-language parsing + +Keyword/regex/exact-string matching on what a user types misses real phrasings, so whole flows silently fail. Route natural language to `execute()`; reserve exact-string branching for explicit slash-commands and structured event `type`s. + +```typescript +// ❌ WRONG - misses "can you sum this up?", "tl;dr", ... +if (message?.payload.text.includes('summarize')) { + /* ... */ +} + +// ✅ CORRECT - the model interprets intent; extract fields with zai +await execute({ instructions: 'Summarize the conversation for the user' }) +``` + ### State Management Mistakes #### Conversation State Access