Skip to content

feat: datasearch tool (Tako) with keyless free-tier fallback - #1175

Open
25eliu wants to merge 43 commits into
paradigmxyz:mainfrom
25eliu:tako-tool
Open

feat: datasearch tool (Tako) with keyless free-tier fallback#1175
25eliu wants to merge 43 commits into
paradigmxyz:mainfrom
25eliu:tako-tool

Conversation

@25eliu

@25eliu 25eliu commented Jul 24, 2026

Copy link
Copy Markdown

Summary

New tools/research/tako/ plugin: datasearch — cited, chart-backed structured data across companies & financial markets, macroeconomics & government data, digital & industry intelligence, sports, polling & live events, and weather & climate, plus web results, via Tako. Coverage is stated by domain rather than by the licensed sources behind it: sources change without the coverage changing, each card carries its own attribution, and naming three made the tool read as narrower than it is. The list has one definition, DOMAINS in _coverage.py, which the CLI help and the tool-directory row render from. Installed as the datasearch CLI; the package/directory keep the tako name.

Commands: available-data (free coverage discovery), search / answer (priced structured+cited results), contents (dataset export), slack-card (render a card as a Slack message), health.

Two backends, one shape

Selected by whether TAKO_API_KEY is configured (never by key value — sandboxes hold only placeholders):

  • With a key → the official tako-sdk against tako.com, full features.
  • Without a key → Tako's free rate-limited hosted MCP (mcp.tako.com), same additive-credential pattern as websearch's Parallel fallback. contents stays key-only.

Both backends return the same shape: searchanswer_markdown (readable ## Tako Data document) + structured cards (image_url, webpage_url, node_ids, exportable, row content) + meta; answeranswer prose + cards; available-datafound/summary/matches. The SDK path renders answer_markdown from its cards; the MCP path reads the hosted tool's two-channel response (readable text + structuredContent). Robust across tako-mcp#187 (bulk payload moving into structuredContent): works before and after that deploys. Every response carries meta.backend (tako:sdk/tako:mcp) and meta.partial_failures.

Sandbox integration

  • Optional replace-mode TAKO_API_KEY bound to X-API-Key on tako.com; read via the secret() placeholder mechanism. mcp.tako.com added to the host allowlist for the keyless path.
  • Firewall-aware routing: inside a sandbox, tool secrets have no env signal, so with HTTPS_PROXY set the client takes the SDK path optimistically and falls back to the free MCP once on a 401 (remembered for the client's lifetime).
  • SDK workarounds (documented in the README): explicit iron-proxy/CA wiring (tako-sdk's urllib3 ignores proxy env vars); per-request timeouts via the generated API (the facade drops them); X-API-Key default header (the SDK's OpenAPI spec omits auth on the beta graph endpoints).

Rendering a card into Slack

slack-card turns a card into a Block Kit message and prints it; search / answer
gain --slack-card, which attaches the same rendered message under
slack_card.message. When a result carries a card the payload also gains
slack_card.hint naming the pipeline that posts it, emitted as the first key
because a search result runs past a hundred lines of JSON and callers routinely
pipe it through head -N.

It renders an image block pointed at the card's own image_url. Slack scales an
image block to the message width and preserves aspect ratio without cropping, so
there is no geometry to fit, and the headline (the card's own description) and
the "Open in Tako" button sit outside the image where they cost no image height.
Nothing here reads, scales, or reformats a value: Tako already chose the axes,
units, and currency, so its render and its description pass through untouched, and
tests pin that. Verified against a live workspace: both the container and flat
image/context/actions layouts post, and Slack keeps the blocks as sent.

Posting deliberately lives in the slack tool, not here. This PR therefore
touches one file outside tools/research/tako/: slack send gains
--blocks-json (a file path, or - for stdin; accepts a bare blocks array or a
whole chat.postMessage body), about ten lines.

datasearch slack-card <card> | slack send <channel> "<fallback>" --thread <ts> --blocks-json -

Posting from this tool was implemented first and did not work. Traced from a real
sandbox: secret("SLACK_BOT_TOKEN") returns the placeholder, iron-proxy allows
egress to slack.com and forwards POST /api/chat.postMessage, but no credential
is substituted, so Slack answers invalid_auth. The slack tool's own health
fails identically in the same sandbox, so this is per-tool secret scoping
(tools/README.md: "tool secrets are scoped") working as intended rather than a
misconfiguration. Moving the send to the tool that already owns the Slack
credential removed ~120 lines here, plus the SLACK_BOT_TOKEN binding and the
slack.com host entry from this tool's manifest, so its permissions match its
purpose again.

Two known limitations, neither introduced here:

  • Rendering is Slack-specific. The sandbox is not told which chat surface a turn
    belongs to, so the hint is phrased conditionally ("if this turn is in a Slack
    thread") rather than suppressed, which would mean never showing it. Discord,
    Teams, Linear and GitHub thread keys all resolve to no Slack target and degrade
    without error; the retrieved data is never affected.
  • Card images render dark only. output_settings.image_dark_mode is accepted and
    returns a distinct card id but does not change the rendered pixels, so a light
    variant is not reachable today. Filed upstream as TAKO-3967.

Notes

  • The keyless MCP backend rebuilds its response dict, which dropped the top-level pub_id/image_url/embed_url pointer the SDK path passes through. It is copied over now, so a response whose cards come back empty still names its chart and both tiers render identically (pinned by a test asserting byte-identical blocks).
  • Tests live in tests/ (no __init__.py) so pytest's module naming doesn't shadow the tako SDK package.
  • Reviewed against AGENTS.md + an adversarial pass; validated live against both tako.com (keyed) and mcp.tako.com (keyless free tier).
  • docs/pages/reference/tool-directory.mdx gains a datasearch row, matching every other tool under tools/research/. The generated docs/public/md/ copy is hand-edited to match rather than regenerated, so unrelated stale generated pages stay untouched.

Test plan

  • uv run --no-project --with pytest --with "tako-sdk>=2.2.6" --with httpx --with python-dotenv --with typer --with rich python -m pytest tools/research/tako/tests/test_client.py -v — 154 tests: coverage helpers, summary builder, available-data pipeline, _sources contract, priced-path shaping, input validation, both-channel MCP parsing, sandbox auth-fallback. No network.
  • python scripts/validate_cli_packaging.py — passes (datasearch → centaur_tool_tako.cli:app).
  • ruff check tools/research/tako — clean, zero findings (CLI imports reordered to the websearch pattern; load_dotenv() now runs after the imports, and the client is imported lazily inside each command so behavior is unchanged).
  • uv build --wheel tools/research/tako — wheel contains only the nine source files; no .env, .venv, or caches leak in.
  • Live keyed (TAKO_API_KEY): datasearch health, available-data, search, answer.
  • Live keyless free tier (mcp.tako.com): available-data, search, answer return real readable content.
  • datasearch --help from an installed wheel — Typer app resolves and lists health, search, answer, contents, available-data.
  • Catalog discovery + one real command from a local Kubernetes sandbox (kind-centaur), per AGENTS.md, run against this PR's head (f292f359) — the local stack's repo-cache tracks this branch, so the sandbox ran the pushed source, not a local copy:
    • centaur-tools listdatasearch /app/tools/research/tako, listed alongside its tools/research/ siblings. This also confirms the catalog name is datasearch, which is the name used in the Tool Directory row.
    • datasearch --help → Typer app resolves through the shim; all six commands listed, including slack-card.
    • datasearch health and datasearch available-data "Nvidia" → real cited data returned through iron-proxy. With no TAKO_API_KEY in this deployment's vault, the client hit a 401 on the SDK path and emitted one line to stderr — tako: API rejected the placeholder credential (no TAKO_API_KEY in the deployment vault); falling back to the free MCP tier — then succeeded on "backend": "tako:mcp". That is the firewall-aware routing and one-shot 401 fallback described above, exercised end to end in-cluster.
    • datasearch search "NVIDIA Corporation Revenue" → documented shape confirmed on the wire: answer_markdown (## Tako Data (1 card)) + cards carrying image_url, webpage_url, exportable, content + meta.backend / meta.partial_failures.
  • ruff check tools/research/tako tools/productivity/slack/cli.py — clean for the tako tool; the slack CLI's 17 pre-existing findings are unchanged before and after this PR's edit (verified by checking out the HEAD version at the same path and re-running).
  • Render-to-post handoff: datasearch slack-card output fed straight into slack send's --blocks-json reader, which accepts it and returns the container block. The reader is unit-tested against a whole chat.postMessage body, a bare blocks array, empty stdin, non-JSON, a missing blocks key, and an empty blocks array.
  • Live Slack workspace: both layouts posted via chat.postMessage; response_metadata shows the blocks were kept, not silently dropped. Slack's ignored_extra_attributes_for_image_block warning is benign and unavoidable — the minimal legal image block produces it, and omitting alt_text to silence it fails outright with missing required field: alt_text.
  • Non-Slack surfaces: the real thread-key shapes for Discord, Linear, GitHub and Teams (taken from each ingress's source) all resolve to no Slack target across both code paths — no exceptions, and cards intact in every case.
  • Posting from a sandbox is currently blocked in the local stack: credential substitution is not applied to any tool there, including the slack tool's own health. Not introduced by this PR and not fixable within it.

@25eliu 25eliu changed the title feat: tako research tool with available-data discovery feat: Tako knowledge search tool for research Jul 24, 2026
- Sources built with conditional kwargs so a skipped source is absent
  from the wire body, not serialized as null (data-count 0 promise)
- data_count=0 with node_ids/strict now raises; negative and >20 counts
  rejected; skipping both sources rejected
- effort/mode/content_format validated pre-network via SDK-derived enums;
  CLI surfaces client ValueErrors as one-line BadParameter messages
- request timeouts via _api._request_timeout (120s priced, 30s graph)
- NER_LABELS/NODE_TYPES derived from SDK enums instead of frozen copies
- other_matches carry node_id; summary marks undrilled hits 'not checked'
  and says so explicitly when no drilled match has coverage
@25eliu

25eliu commented Jul 24, 2026

Copy link
Copy Markdown
Author

Addressed all six findings from @robertabbott's review in 698c16c (replying here because the review threads aren't visible to the API yet — will follow up inline once they are):

  1. found only reflects the top-2 drill / other_matches lack node_id — did both suggested fixes: other_matches now carry node_id, and the summary marks them "Also matched (not checked)". When no drilled match has coverage, the summary states outright that the unchecked hits are not proof Tako lacks data and points at --types/--label narrowing or search --node-id. Verified live: rank-3+ hits now come back pinnable.

  2. Frozen NER_LABELS/NODE_TYPES copies — switched to deriving from the SDK enums (tuple(l.value for l in NerLabel) etc.); the frozen copies weren't deliberate. Same treatment applied to the new option validators (SearchEffortLevel, ContentsDeliveryMode, ContentsFormat), with tests asserting the derived tuples track the enums.

  3. --effort medium / --data-count 50 uncaught tracebacks — all enum and range options are now validated in the client before any network call (ValueError), and the CLI translates those into one-line BadParameter messages. Confirmed no tracebacks for effort/mode/content_format/count violations.

  4. No request timeout — went through _api as suggested; it's the same escape hatch the auth-header workaround already uses. Every operation now passes _request_timeout: 120s for search/answer/contents, 30s for graph calls, both constructor-overridable. README maintainer note updated.

  5. Sources(data=None) serializing "data": null — fixed with conditional kwargs exactly as proposed; a skipped source is now genuinely absent from the wire body. Test asserts "data" not in to_dict() and not in model_fields_set.

  6. data_count=0 silently dropping node_ids/strict; negatives folded into skip — both now raise: data_count=0 with pinning is rejected as contradictory, counts are range-checked 0–20 (negatives rejected), and skipping both sources is rejected too.

69 unit tests pass (up from 54); live free smoke (health, available-data "tesla") green.

@claude-address

25eliu added 9 commits July 26, 2026 12:02
Without a TAKO_API_KEY, search/answer/available-data route to the free
rate-limited MCP at mcp.tako.com (scaffold ahead of the server-side tier
launch; anonymous calls surface a clear message until then). contents
stays key-only. Same additive-credential shape as websearch's Parallel
MCP fallback: routing via ctx.secrets membership, meta.backend +
meta.partial_failures on every response, TAKO_API_KEY becomes an
optional_secret, and mcp.tako.com joins the host allowlist.
…de deep effort

Cross-checked _mcp.py against the tako-mcp worker tool schemas
(workers/src/tools/) rather than assumptions:

- tako_search's effort enum is fast/instant only, so effort=deep on the
  keyless path would hit a schema rejection server-side. The backend now
  records a meta.partial_failures entry and runs the search at the
  default instead (same degrade pattern as the answer path and the
  websearch tool's MCP mode fallback).
- other_matches from the hosted tako_available_data carry name/type only
  (no node_id, unlike this tool's keyed graph pipeline). Docstrings,
  CLI help, and README now state the difference instead of promising
  node_id on both backends.
- README documents the differing backend defaults (MCP: 10 results with
  inline row previews; API: 5 pointer cards) and records which schema
  facts were verified and that the final keyless contract lands with the
  unmerged tako-mcp anonymous-tier PR.

Tests: updated the passthrough case to a supported effort and added a
deep-degrade case asserting the argument is omitted and flagged
(80 passed).
…ract

The server side now exists: TakoData/tako-mcp#171 serves anonymous /mcp
as a fail-closed free tier (3-tool surface, 10 metered tools/call per
minute per client IP, 429 + JSON-RPC error + Retry-After when over).
Align the backend with that contract:

- 429 -> McpRateLimited, surfacing the server's own error.message
  verbatim (so the stated limit can never drift) plus the Retry-After
  hint and the TAKO_API_KEY remedy. Guarded on both the tools/call and
  handshake paths.
- Drop the client-minted Mcp-Session-Id: the Worker is stateless, never
  issues one, and metering is per client IP, not per session. Requests
  now carry no auth or session headers at all — header ABSENCE is what
  routes to the free tier (empty/malformed would 401).
- 401 message updated: it now means the tier is disabled on that
  endpoint (over-limit is 429, not 401).
- README: per-IP metering caveat for NATed sandbox fleets (shared
  10/min bucket -> fallback, not fleet capacity), and the verified
  contract note now cites the PR.

Tests: 429 with server envelope, 429 with unparseable body, and a
header-absence guard (no authorization / mcp-session-id on any
request). 83 passed; ruff at the pre-existing baseline.
Tool secrets have no env signal inside a sandbox (placeholders are
swapped by iron-proxy on the wire), so _is_configured cannot see the
deployment vault. With the firewall active the client now tries the SDK
path first and degrades to the free MCP tier once on 401/403, keeping
zero-config deployments working while keyed ones are unaffected.
Routing experiment: the catalog lists bare names, so 'research'
self-describes at discovery where 'tako' required reading --help.
Help text now leads with web search plus the concrete licensed data
domains (S&P Global financials, FRED macro, SimilarWeb traffic).
Command name flows from _coverage.TOOL_COMMAND so the model-facing
summary strings stay consistent.
Two routing levers for data queries that were defaulting to websearch:
- the sandbox prompt's research-path guidance now names `research` for
  financials/market/macro/web-traffic lookups (the prompt previously
  recommended only websearch, in-context, before any catalog reading)
- websearch's own --help cross-references `research` for those domains,
  intercepting at the moment of misrouting

Cross-tool + prompt change; flagged for reviewer attention.
- command renamed research -> datasearch (project.scripts + TOOL_COMMAND);
  package/directory keep the tako name
- reverted the websearch --help cross-reference (no cross-tool edit)
- system-prompt routing line rewritten concise: financials/markets/
  economy/web-traffic or web-live data -> datasearch
The live free tier returns a two-channel MCP response (readable text
document + structuredContent payload), which the old fallback discarded —
keyless search/answer/available-data returned empty. Now:

- _mcp.py reads both channels: text -> answer_markdown/summary, structured
  -> cards/answer/found. Robust across tako-mcp#187 (bulk payload moving
  into structuredContent): works pre- and post-deploy.
- SDK path gains answer_markdown (rendered from cards) so both backends
  return the same shape (matches the websearch answer_markdown pattern).
- removed dead widget-stripping helper; tests model the real two-channel
  envelope; ruff clean (bar accepted load_dotenv/sys.path E402s).

Verified live against mcp.tako.com: available-data/search/answer all
return real readable content keyless.
@25eliu 25eliu changed the title feat: Tako knowledge search tool for research feat: datasearch tool (Tako) with keyless free-tier fallback Jul 29, 2026
Remove design docs (docs/superpowers/) and revert the SYSTEM_PROMPT.md
routing line — neither is standard tool-PR content per AGENTS.md, which
scopes tool contributions to the plugin under tools/. The datasearch
routing preference, if wanted upstream, belongs in a separate prompt PR
or a deployment persona overlay.
@gakonst
gakonst enabled auto-merge (squash) August 5, 2026 17:39
25eliu added 2 commits August 10, 2026 17:24
Every other tool under tools/research/ has a row in the Tool Directory
reference; datasearch was missing. The public/md copy is hand-edited to
match rather than regenerated, so unrelated stale generated pages stay
untouched.
load_dotenv() ran before the module imports, which tripped four E402s.
The client is imported lazily inside each command, so moving the call
below the imports keeps the same behavior and clears ruff.
auto-merge was automatically disabled August 11, 2026 00:24

Head branch was pushed to by a user without write access

25eliu added 12 commits August 11, 2026 10:41
The SDK path returns the whole /v3/search body, which carries a top-level
pub_id/image_url/embed_url pointer to the response's lead card. The MCP
backend rebuilds its response dict and dropped those keys, so the free
tier lost a chart pointer the paid tier keeps. It matters most in the
pre-187 shape, where `cards` comes back empty and that pointer is the only
thing naming the chart.

Copied through when the server sends them, so both backends expose the
same pointer. Additive: no existing key changes.
`datasearch slack-card` turns a search result into a Slack message and
optionally posts it into the thread the turn belongs to. Presentation
only: every card already carries its rendered chart in `image_url`, so
this makes no Tako API call.

Uses an `image` block. Slack scales it to the message width and preserves
aspect ratio without cropping, so there is no geometry to fit. The
headline (the card's own `description`) and the "Open in Tako" button sit
outside the image, costing no image height, and interactivity is delegated
to the button. The `video` block would render a live iframe but fixes the
frame to a small box with no size controls, crops taller cards, loads only
on click, is dropped from ephemeral messages, and needs links.embed:write
on both tokens plus a registered unfurl domain.

Nothing here reads, scales, or reformats a value: Tako already chose the
axes, units, and currency, so its render and its description pass through
untouched. Tests pin that.

Both backends render identically, including the empty-`cards` case that
falls back to the top-level pointer.

`--post` posts directly with httpx rather than delegating to the slack
tool: tools run under `uvx --from <project_dir>`, so a sibling tool's
package is not importable. httpx honors HTTPS_PROXY and SSL_CERT_FILE from
the environment, so no explicit iron-proxy wiring is needed. This binds
SLACK_BOT_TOKEN and widens this tool's egress to slack.com, called out in
the manifest comment; dropping both disables only `--post`, and the
command still prints its payload for another tool to send.

Verified against a live workspace: both layouts post, and the container
block is kept rather than silently dropped.
The help and README named three licensed sources and trailed off with "and
more", which read as a narrower tool than it is: the routing guidance covers
roughly a dozen domains, so a sports or elections question could get routed
to websearch on the strength of a help string.

States coverage as five domains instead, and points at `available-data` as
the authority on whether a specific entity or metric is covered. Sources
are an implementation detail that changes without the coverage changing, and
each card already carries its own attribution.

The list has one definition, DOMAINS in _coverage.py, which the CLI help
renders from and the README points at, so the two cannot drift again.
`slack-card` existed but nothing connected "this result has a chart" to
"show it", so a data question in Slack returned text and the card never
appeared unless the caller already knew the command.

Two additions:

- `--slack-card` on `search` and `answer` posts the lead card into the
  thread the turn belongs to, so one call retrieves and renders.
- Without the flag, a result that has a card in a Slack thread gains a
  `slack_card.hint` naming the command that renders it.

Both are conditional on being in a Slack thread with a renderable card, so
Discord, Teams, and direct API turns are untouched, and web-only results
stay unchanged.

A failed post never loses the retrieved data: the result still prints and
the failure lands in `slack_card.error`. Posting is a side effect of a read
command, so it stays opt-in behind the flag rather than firing on every
search.
`--slack-card` and the hint both gated on CENTAUR_THREAD_KEY, which is unset
in every live sandbox: warm-pool pods start before a thread claims them and
pod env cannot be changed afterwards. So neither ever fired, on search or on
answer, and the flag was silently inert.

The destination was never in the environment to begin with. It arrives in the
turn's prompt, under session_context.slack.channel_id / .thread_ts, and
`slack upload` already takes it as arguments; this now follows that precedent
instead of sniffing env.

- `--channel` / `--thread` on search and answer, passed through to the post.
- The hint no longer requires a resolved destination. It cannot tell which
  chat surface a turn belongs to, so it is phrased conditionally and names the
  two options to pass; gating on the environment meant never showing it.
- `--post` without a destination now says which values to pass and where they
  come from, rather than reporting an unset variable the caller cannot set.
- CENTAUR_THREAD_KEY stays as a fallback for a dedicated sandbox that has it.

Verified against a live workspace with the variable unset: the hint renders,
and an explicit --channel posts.
The note was appended as the last key, which put it on line 156 of a
160-line search result. Callers pipe this through `head -80` or `head -100`,
so the hint was cut off before it was ever read: the reason a data question
in Slack still produced no card even after the destination fix.

Emitting it as the first key puts it on line 2. Two tests pin it: the note is
the first key, and it survives truncation of a realistic multi-card result.
The fixtures carried a real workspace team id, a real private channel id,
and real message timestamps captured while testing. AGENTS.md prohibits
adding chat workspace identifiers and asks for neutral placeholders in new
examples.

Switched to the placeholders the sibling test_slack.py already uses
(T0123ABC, C0456DEF, 1754870000.001200) so both files read the same way.
The directory row still listed the licensed sources, contradicting the
tool's own help and README after 5ab59e5. Same domain wording as DOMAINS.
`send_message` in the client has taken a `blocks` argument for a while, but
the CLI had no way to pass one, so a tool that renders Block Kit had no way
to post it. `--blocks-json` takes a file path or `-` for stdin.

Accepts either a bare blocks array or a whole `chat.postMessage` body with a
`blocks` key, so a renderer's output pipes straight in:

    datasearch slack-card <card> | slack send eng-ai "Apple revenue" --blocks-json -

`message` stays required: with blocks it is the notification and old-client
fallback text, which Slack needs regardless.

Posting lives here rather than in the rendering tool because this tool
already owns the Slack credential. Tool secrets are scoped per tool
(tools/README.md), so a Tako tool holding a Slack bot token would cut
against that.
Posting from this tool never worked. Traced from a real sandbox:
secret("SLACK_BOT_TOKEN") returns the placeholder, the proxy allows egress to
slack.com and forwards the request, but no credential is substituted, so Slack
answers invalid_auth. That is the scoping working as intended rather than a
misconfiguration: tool secrets are scoped per tool (tools/README.md), and this
is a Tako tool.

So posting moves to the tool that already holds the Slack credential, and this
one renders only:

    datasearch slack-card <card> | slack send <channel> "<fallback>" --blocks-json -

Removed: post_message, SlackPostError and its error mapping from _slack.py;
_slack_destination, --post, --channel and --thread from cli.py; the
SLACK_BOT_TOKEN binding and slack.com egress from the manifest, so this tool's
permissions match its purpose again. Net 120 lines lighter here for ten in the
slack tool.

`--slack-card` on search and answer now attaches the rendered message under
slack_card.message instead of sending it, keeping the one-call convenience, and
the payload pins no channel or thread since the posting tool supplies those.
The hint names the full pipeline.
Tako serves card images dark by default and stamps `dark_mode=true` on
`image_url` / `dark_mode=auto` on `embed_url`. Both are wrong for a Slack
message, which renders on each reader's own theme: a dark chart in a light
thread reads as a black box, and `auto` resolves against the fetcher's
environment rather than the reader's, so it is effectively arbitrary. One fixed
appearance beats a coin flip, and light is the safe one.

New _theme.py owns the rewrite, applied at two layers because neither alone is
sufficient:

- client.py, at the response boundary, so every card that leaves the tool is
  already light and a new surface needs no theme code. Wrapped inside the
  via_mcp closure so the keyless path and the 401-fallback path are covered by
  one edit, and around the SDK lambdas for the keyed path.
- _slack.py, in card_urls. This is the one that mattered: `slack-card <pub_id>`
  builds {"card_id": pub_id} with no image_url, so the payload pass has nothing
  to rewrite and the URL is reconstructed unparameterized -- which renders dark.
  The response-boundary fix alone left the primary flow dark.

Verified against production on 2026-08-11 (card vjvljW2ZZ3ImEsq1P-nI):
/api/v1/image/<id>/ with no parameter and with dark_mode=true return
byte-identical dark PNGs, dark_mode=false returns a genuinely light one, and
/embed/ likewise distinguishes true from false while serving auto as false. So
the parameter is honored today. The trap is that a *missing* parameter means
dark, so it has to be added, not merely overwritten -- the previous note in
_slack.py concluded the opposite from testing only dark_mode=true, and is
corrected here.

webpage_url is deliberately untouched: that link opens the live card on
tako.com, which should follow the visitor's own theme.

light_mode_url is idempotent, rewrites tako.com hosts only (a lookalike host or
a web result's url is returned unchanged), and apply_light_mode returns a copy
rather than mutating the payload. Six tests in test_slack.py pinned the old
pass-through behaviour and are updated to the new intent; the "never a rebuilt
image" invariant still asserts host, path and card id survive, and now also
pins the theme.
Review pass over this PR turned up three real defects, two of them in the
light-mode change from the previous commit.

**answer_markdown contradicted cards[].** `_add_search_markdown` writes
`chart: <image_url>` into the prose it renders from the cards, and the theme pass
ran around it rather than inside it, so on the SDK search path the prose quoted
the original dark URL while `cards[]` carried the light one. Reordered so the
pass runs first and the markdown is rendered from already-light cards.

**The keyless path never pinned the prose at all.** `_mcp` forwards the hosted
tool's own document, which names image URLs with no `dark_mode` parameter -- and
a bare URL renders dark, so `answer_markdown` pointed at dark charts on the free
tier no matter what `cards[]` said. Added `light_mode_markdown`, a text-level
rewrite of Tako image/embed URLs, applied to `answer_markdown` and to `answer`.
`answer` is normally a one-line synthesis, but `_mcp.answer` falls back to the
whole text channel on servers predating tako-mcp#187, and that document does name
image URLs, so it is covered rather than assumed clean. `/card/` links are left
alone, matching the existing `webpage_url` rule. The URL pattern stops before
markdown and sentence punctuation so a link inside `[text](url)` is not swallowed
with its delimiter.

Verified both paths end to end: SDK from a real sandbox, keyless against
mcp.tako.com -- markdown URLs all light, quoting cards[0] verbatim, `/card/`
links untouched.

**`slack send --blocks-json` silently dropped requester attribution.** Slack
renders `blocks` when present and demotes `text` to notification/fallback, so the
attribution `send_message` appends to `text` was invisible in the message body.
Latent until now (`blocks` was unreachable from the CLI); this PR makes it live.
Attribution now also rides as a trailing context block, built on a copy so the
caller's list is not mutated, and still suppressed by `--no-attribution`.

Tests: `light_mode_markdown` and the markdown/cards agreement, the `answer`
fallback, attribution-with-blocks including the no-mutation guarantee, and
`_read_blocks`, which had no coverage at all despite being this PR's new entry
point (bare array, full postMessage body, stdin, and each rejection path).
191 tako + 89 slack green; ruff clean on every file touched.

Pre-existing lint and format drift in slack/cli.py and slack/tests/test_cli.py is
left alone per AGENTS.md -- none of it is on lines this PR touches.
@25eliu

25eliu commented Aug 11, 2026

Copy link
Copy Markdown
Author

Follow-up: the card renders, but nothing tells the agent to post it

Flagging a deliberate gap so it is a decision on the record rather than a surprise.

What works. datasearch search/answer return the cards, and when a result carries one the payload gains a slack_card hint naming the exact pipeline, on both the keyed and keyless backends:

if this turn is in a Slack thread, post this chart with:
datasearch slack-card 2DCtQtFLAXH7nxEe_50X | slack send <session_context.slack.channel_id> "<fallback>" --thread <session_context.slack.thread_ts> --blocks-json -

What does not. The agent usually ignores it. Traced on a real Slack turn (keyless sandbox, meta.backend: tako:mcp) — the complete list of commands it ran was:

datasearch --help 2>&1 | head -30
datasearch answer "Microsoft annual revenue 2021 2022 2023 2024 2025" 2>&1

It read the hint and stopped. No slack-card, no slack send. No rate limit, no auth error, cards present in the output. Keyed behaves the same, so this is not a free-tier gap.

Why. Nothing in the agent's instructions asks for the chart to be rendered into the thread. services/sandbox/SYSTEM_PROMPT.md has a routing line telling the agent to use datasearch for real-world metrics, but no render obligation, so the only signal is a conditional hint inside a large JSON payload — a weak one competing with "answer the question." That is why the behaviour is inconsistent rather than absent.

Deliberately not fixed here. Making it reliable needs roughly one line of agent instruction ("when a datasearch result carries a slack_card hint and the turn is in Slack, run the command it names before answering"). Two candidate homes:

  1. services/sandbox/SYSTEM_PROMPT.md — reliable, but 30081e0 in this branch removed a datasearch line from that file precisely to keep this PR tool-scoped per AGENTS.md, and its commit message says the routing preference "belongs in a separate prompt PR or a deployment persona overlay." Re-adding it here would reverse that. It also needs an agent-image rebuild to take effect: the prompt is COPY'd to /home/agent/AGENTS.md at build time, unlike tools, which come from the repo cache.
  2. A persona overlay — what 30081e0 itself nominates, and it needs no image rebuild.

Recommendation: keep this PR tool-scoped and do (1) or (2) separately. The tool side needs no further change either way — the hint is already emitted with the right card id and the right pipeline.

`ThreadTarget` and `thread_target()` resolved the Slack thread a sandbox turn
belonged to, so `--post` could default its destination. f292f35 moved posting to
the `slack` tool and removed `--post`/`--channel`/`--thread`, which left the
parser, its regex, and the `CENTAUR_THREAD_KEY` read reachable only from their own
tests. Removed all three plus TestThreadTarget.

This also takes the last `os.environ` read out of _slack.py, so the module is now
pure rendering with no environment coupling at all -- easier to justify living in
a research tool, and one less thing for a reviewer to ask about.

185 tako tests green (was 191; the 6 removed covered only the deleted parser).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants