From 06b49b5e2e2437da8febfdb139892382144d3d06 Mon Sep 17 00:00:00 2001 From: Alex Skatell Date: Wed, 13 May 2026 17:12:47 -0400 Subject: [PATCH] docs(skills): adopt composite query audit contract + ban bash heredocs Bundled skill docs were still telling agents to author a hand-rolled composite SQL string for the standard pipeline audit. Now that `topline --agent query audit|snapshot|freshness` ship as composite warehouse commands (PR #8), the documented contract collapses to: 1. query doctor 2. query audit --pipeline ... --since ... --status open 3. answer Hard ceiling drops from 4 calls to 3. Raw `query sql` is reserved for non-standard analytics only. Adds an explicit pitfall: bash heredocs around `query sql` (`SQL=\$(cat <<'SQL' ... SQL)` or `--sql "\$(cat <'` — ONE call returning open count/value, stage rollup, activity by channel/direction, moved/created/closed deals in window, top active deals. If the window is custom/relative and not already known, prepend a single `date` call before this step. +1. `topline --agent query doctor` — readiness probe. JSON: `queryTokenPresent`, `schemaReachable`, `tableCount`, `missingTables`, `recommendation`. If `queryTokenPresent` is false, `schemaReachable` is false, or any expected table is missing, stop and report the readiness gap. Missing tables/views are `os-mcp` coverage bugs; surface them in the final answer. +2. `topline --agent query audit --pipeline PIPELINE_ID --since WINDOW --status open` — one composite call returning `freshness`, `snapshot`, `activity` (with `unique_touches`), `deals`, and `movement`. Default `--since this-week-et` for "this week"; the CLI resolves the window. 3. Answer. -**Hard ceiling: 4 tool calls after skills load** (date if needed + readiness probe + composite SQL + answer). Banned in the default flow: +**Hard ceiling: 3 tool calls after skills load** (doctor + audit + answer). Banned in the default flow: +- Raw `topline --agent query sql --sql ...` for standard pipeline audits — `query audit` already covers the shape. +- `query schema`, `query explain`, or per-table freshness SQL before `query audit`. The audit payload's `freshness` field is enough. - `pipeline audit`, `opportunities search`, `conversations search`, per-conversation message fetches. -- `python3` / `execute_code` / any subprocess wrapper around `topline`. The CLI returns JSON; parse it directly. Reshape data in SQL with a final `SELECT`, not in Python. +- `python3` / `execute_code` / any subprocess wrapper around `topline`. The CLI returns JSON; parse it directly. +- **Bash heredocs around `query sql`**: `SQL=$(cat <<'SQL' ... SQL)` or `topline --agent query sql --sql "$(cat <'` — ONE call, returns: open count/value, stage rollup, activity by channel/direction, moved/created/closed deals in window, top active deals. If the window is custom/relative and not already known, prepend a single `date` call before this step. +1. `topline --agent query doctor` — readiness probe. If `queryTokenPresent` is false, `schemaReachable` is false, or any expected table is missing, stop and report the readiness gap. Missing tables/views are `os-mcp` coverage bugs; surface them in the final answer. +2. `topline --agent query audit --pipeline PIPELINE_ID --since WINDOW --status open` — one composite call returning `freshness`, `snapshot`, `activity` (with `unique_touches`), `deals` (per-deal touch breakdowns), and `movement`. Default `--since this-week-et` for "this week"; the CLI resolves the window so a separate `date` call is not needed for that phrase. 3. Answer. -**Hard ceiling: 4 tool calls after skills load** (date if needed + readiness probe + composite SQL + answer). The following are explicitly banned in the default flow: +**Hard ceiling: 3 tool calls after skills load** (doctor + audit + answer). The following are explicitly banned in the default flow: +- Raw `topline --agent query sql --sql ...` for standard pipeline audits — the composite `query audit` already covers the shape. +- `query schema`, `query explain`, or per-table freshness SQL before `query audit`. Use `query doctor` for readiness and rely on the audit payload's `freshness` field. - `pipeline audit`, `opportunities search`, `conversations search`, per-conversation message fetches. -- `python3` / `execute_code` / any subprocess wrapper around `topline` calls. The CLI already returns JSON; parse it in the answer, not in a Python loop. Reshape data inside the SQL with a final `SELECT`, not in a wrapper script. -- `skill_manage` edits to this skill or `topline-os-crm-audits` during the audit. The contract is **read-only during execution**. If the skill is wrong, finish the current run honestly (or stop and disclose the gap), then propose the edit in a separate turn. Mid-run skill edits invalidate the test trace because the documented contract is no longer what the agent followed. - -`query doctor` replaces the prior ad-hoc freshness/token probe — it is faster, deterministic, never prints the token, and surfaces both auth/readiness and warehouse coverage in one JSON payload. Only run a separate freshness SQL (`MAX(_synced_at)` per table) if the user explicitly asks about sync lag. +- `python3` / `execute_code` / any subprocess wrapper around `topline` calls. The CLI already returns JSON; parse it in the answer, not in a Python loop. +- **Bash heredocs around `query sql`**: `SQL=$(cat <<'SQL' ... SQL)` or `topline --agent query sql --sql "$(cat <= 'YYYY-MM-DDT00:00:00-04:00') SELECT COALESCE(type, 'unknown') AS activity_type, COALESCE(direction, 'unknown') AS direction, COUNT(*) AS touches, COUNT(DISTINCT contact_id) AS contacts_touched, MIN(date_added) AS first_touch, MAX(date_added) AS last_touch FROM week_messages GROUP BY activity_type, direction ORDER BY touches DESC LIMIT 20" +topline --agent query explain --tables opportunities,pipeline_stages,messages,contacts,call_events,appointments +topline --agent query sql --sql 'SELECT status, COUNT(*) AS n FROM opportunities GROUP BY status ORDER BY n DESC' ``` +Raw SQL is for analytics the composite commands cannot express (e.g. unusual rollups, cross-pipeline funnels, custom user-supplied joins). Keep it inline; do not assemble multi-line SQL in a bash heredoc. + Rules: - The command calls the hosted `Topline-com/os-mcp` `/query/api/*` endpoints and inherits MCP SQL safety: one `SELECT` / `WITH ... SELECT`, exposed tables only, 5,000-row cap. -- Use SQL first for counts, joins, stage/value snapshots, contact activity rollups, and funnel analytics. Use REST/CLI action commands for live mutations, exact object drilldowns, or when synced warehouse freshness is uncertain. +- Use SQL first for counts, joins, stage/value snapshots, contact activity rollups, and funnel analytics that the composite commands do not cover. Use REST/CLI action commands for live mutations, exact object drilldowns, or when synced warehouse freshness is uncertain. - Do not print query tokens. If missing, ask the user to mint/configure a connection-bound query token — do not silently fall back to REST and present it as the SQL-first result. ## Pipeline audit (diagnostic / one-off only — NOT the analytics default) @@ -98,7 +118,7 @@ topline --agent pipeline audit \ Confirm `activityJoinIncluded: true` in the JSON before trusting any zero-activity result. If that field is missing or false, the installed CLI is old or the audit was run with `--skip-activity`; treat the output as snapshot-only. -Do **not** run this after a successful SQL audit "to verify" — cross-verification is a drilldown subroutine, not the default. +Do **not** run this after a successful `query audit` "to verify" — cross-verification is a drilldown subroutine, not the default. Flags: @@ -129,24 +149,26 @@ topline raw request GET /opportunities/search --query '{"pipelineId":"PIPELINE_I - Prefer `--agent` for concise, token-efficient output. - Add `--mask-pii` when sharing outside a private internal context. -- Use Python or SQL for arithmetic, grouping, and joins; never do CRM math mentally. +- Use composite query commands or inline SQL for arithmetic, grouping, and joins; never do CRM math mentally. - Avoid markdown tables in chat replies (Discord/WhatsApp). Use bullets. - Keep sales reporting separated into: activity, movement, hygiene, next action. ## Common pitfalls -1. **Starting CRM analytics with REST when SQL is available.** If `TOPLINE_QUERY_TOKEN` is configured and the task is a rollup/audit/count/join, use `topline --agent query sql` first. REST pagination and endpoint fan-out are fallback paths, not the default analytics path. -2. **Running both SQL AND `pipeline audit` for the same question.** The default audit ends after the SQL composite. Don't add `pipeline audit` "to verify" — cross-verification is a drilldown subroutine, not the default. Movement is already covered by the composite SQL via `last_stage_change_at` / `last_status_change_at` / `created_at`. -3. **Auto-falling back to REST when SQL is partial.** Hard rule: default audit ends after the SQL composite. Disclose any coverage gap in the answer; the user can ask for a live re-run if it matters. This keeps `os-mcp` view gaps visible as bugs instead of papered over. +1. **Starting CRM analytics with REST or raw SQL when a composite command is available.** If `TOPLINE_QUERY_TOKEN` is configured and the task is a standard pipeline rollup/audit, use `topline --agent query audit` (or `query snapshot` / `query freshness`) first. REST pagination, endpoint fan-out, and hand-written SQL are fallback/advanced paths. +2. **Running both `query audit` AND `pipeline audit` for the same question.** The default audit ends after `query audit`. Movement is already included in `query audit.movement`; don't add `pipeline audit` "to verify." +3. **Auto-falling back to REST when SQL is partial.** Hard rule: default audit ends after `query audit`. Disclose any coverage gap in the answer; the user can ask for a live re-run if it matters. This keeps `os-mcp` view gaps visible as bugs instead of papered over. 4. **Mislabeling SQL/native disagreements as "sync lag".** Only call it lag when `_synced_at` proves lag. If SQL is fresh but a view is missing a UNION (e.g. appointments not yet in `contact_timeline`), say the SQL surface is incomplete for that metric (`os-mcp` coverage gap) and stop. -5. **Assuming opportunity updates equal activity.** Calls/SMS/email live in conversations/messages and may not move `updatedAt` or stage fields. +5. **Assuming opportunity updates equal activity.** Calls/SMS/email live in conversations/messages/`call_events` and may not move `updatedAt` or stage fields. 6. **Skipping stage lookup.** Stage IDs are opaque; map them through `opportunities pipelines` (or a SQL join on `pipeline_stages`) before presenting. 7. **Printing secrets or full PII.** PITs, query tokens, phone numbers, and credentials never belong in logs, memory, or summaries. 8. **Silent writes.** For mutating commands, get explicit approval and state the exact contact / opportunity / action first. 9. **Treating masked `--agent` JSON as machine-parseable for pagination.** `--agent` enables PII masking and may replace numeric pagination fields like `startAfter` with `[PHONE]`, which makes the output invalid JSON. For internal follow-up scripts, use unmasked CLI output into temp files, parse locally, and delete the files before finishing. -10. **Wrapping `topline` in `python3` / `execute_code` / `subprocess.run` to "verify" or "reshape" CLI output.** The CLI already returns JSON; parse it directly in the answer. Python wrapper loops are the new shape of the old REST-fan-out anti-pattern — they blow the 4-call ceiling, hide the actual data path, and produce nothing the SQL composite did not already return. If you genuinely need to reshape data, do it in the SQL itself with a final `SELECT`, not in Python around the CLI. -11. **Editing this skill (or `topline-os-crm-audits`) mid-audit via `skill_manage`.** The contract is read-only during execution. If the skill is wrong, finish the current run honestly (or stop and disclose the gap), then propose the edit in a follow-up turn. Mid-run edits invalidate the test trace because the documented contract is no longer what the agent followed. +10. **Wrapping `topline` in `python3` / `execute_code` / `subprocess.run` to "verify" or "reshape" CLI output.** The CLI already returns JSON; parse it directly in the answer. Python wrapper loops are the new shape of the old REST-fan-out anti-pattern. +11. **Wrapping `query sql` in a bash heredoc.** `SQL=$(cat <<'SQL' ... SQL)` or `topline --agent query sql --sql "$(cat <