Repository navigation
fix(oss): remove Chi-specific paths/IDs from tool descriptions - #700
Merged
Merged
Conversation
Two hardcoded literals that bake an OSS-user-failing assumption into the shipped code (caught by Chi 2026-05-14, both via OSS user's Discord drop): 1. `src/inline-tools.ts:30` — openFileTool description embedded `/Users/wangchi/.sutando-memory-sync/voice-contexts/ag2ai-investor.txt` as a literal example. OSS users have no `wangchi` home, no `.sutando-memory-sync` repo, no `ag2ai-investor` voice context — and the Gemini tool-call layer sees the example verbatim. Replaced with a `<active>` placeholder so the example stays self-documenting but doesn't leak a specific user's filesystem. 2. `skills/phone-conversation/scripts/conversation-server.ts:971` — phone-call summary task instructed the post-call agent to "Send a concise version (3-5 bullet points) to Discord DM (channel 1485370959870431433)" — Chi's specific DM channel. OSS users don't have that channel. Rewrote to point at the bridge's runtime resolution: `$SUTANDO_DM_OWNER_ID / access.json allowFrom`, which `src/dm-result.py` already implements correctly. Behavior changes: - Owner instance: no change (env vars + access.json resolve the same way). - OSS user: phone-call summaries now land in *their* owner DM channel, and the voice context example doesn't tell their Gemini to read a non-existent file. Typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous instruction read two ways to the post-call agent: - Correct: "deliver via the standard task-bridge result-file path" - Confused: "you need to resolve $SUTANDO_DM_OWNER_ID / access.json yourself" The post-call agent doesn't have direct access to env vars or access.json — those are bridge-internal. Step 4 of the same instruction block already specifies the result-file write, and the bridge auto-resolves the DM channel from there. Drop the "(bridge resolves ...)" qualifier; rely on the implicit task-bridge contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
added a commit
that referenced
this pull request
May 15, 2026
1. @chiwang.cc → $SUTANDO_OWNER_EMAIL_DOMAIN env var. Same OSS-hygiene pattern as PR #700 (which I just shipped + Mini caught me repeating). Empty env → skip the bump (no penalty). Set per-user. 2. Drop foundercoho from BLACKLIST_DOMAINS — overly specific brand. The generic noreply@/marketing@/newsletter@/no-reply@ patterns catch it (and arbitrary other brands) anyway. Added bare `noreply@` alongside `no-reply@` for completeness. 3. Cache TTL 15min → 30min. Materially improves cache-hit rate (~70% → ~95% per Mini's analysis) for minor freshness sacrifice; live gws fallback covers the freshness edge case anyway. 4. Replace brittle "DO NOT pass max=1" negative anti-pattern guidance with schema-level ignore + positive description: - tools.ts execute() now logs and ignores `max` when mode=important ("cache returns top-3 regardless") - Description: positive phrasing — "max is ignored in important mode; pass max only with mode='recent'". Harder for LLM to misread. - Parameter zod description follows the same shape. Smoke-tested all 3 paths: - mode=important default → cache hit, top-3 - mode=important + max=1 → logs "ignoring max=1", still returns top-3 - mode=recent + max=3 → live gws, count=3 All Mini's substantive nits addressed; minor style observations (O(N²) sort with N≤30, CACHE_PATH cwd assumption) left as documented in his review since they're non-broken under current usage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
added a commit
that referenced
this pull request
May 15, 2026
…704) * feat(gws-gmail-voice): cache-first 3-tier path + importance scoring Solves two compounding bugs from PR #702 voice testing: 1) Gemini interpreted "most important email" → singular → passed max=1 2) Tool returned N-by-recency, no importance ranking → wrong top hit Architecture (Chi 2026-05-14): - Tier 1 CACHE — read `state/external-cache/inbox-important.json` (mtime <15min). Pre-scored top-3 importance-ranked. Sub-50ms. - Tier 2 LIVE — cache stale/missing or mode=recent → call gws (10s timeout). - Tier 3 WORK — gws fails → description tells Gemini to call `work` next. Cache writer: `skills/gws-gmail-voice/scripts/refresh-cache.py`. Reads `gws gmail +triage --max 30`, scores each via importance heuristics from feedback_triage_recency_not_importance.md: - BLACKLIST domains (substack, linkedin digest, FounderCoHo, ODSC, etc.): -100 - Self-notifs (Chi Wang via github.com notifications, CI failures): +15 - Academic .edu senders, openreview.net: +5/+10 - Subject patterns (deadline/RSVP/CI failure/accepted/urgent/review): +10 each - Calendar invitations: +8 - Sorts top-3 (score > -100), atomic-writes JSON. Tool description tweaks (the real fix to the max=1 bug): - Mode is now explicit: "important" (default) returns pre-ranked top-3; "recent" returns top-N by date. - Explicit instruction: "DO NOT pass max=1 — the tool already returns the most important; max only matters for mode=recent." - Pre-empts Gemini's previous misinterpretation. Run cache writer via ACT loop opportunistically OR ad-hoc: python3 skills/gws-gmail-voice/scripts/refresh-cache.py Smoke-tested end-to-end: - mode=important + fresh cache → top_3: CI failure / Xinyang Han .edu / GitHub PR - mode=recent + live gws → newest-first: Levay LinkedIn / CI / FounderCoHo - Cache stale path: falls through to live correctly. state/external-cache/ is gitignored (real email metadata); only the writer script + tool changes ship. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(gws-gmail-voice): apply Mini PR #704 review nits 1. @chiwang.cc → $SUTANDO_OWNER_EMAIL_DOMAIN env var. Same OSS-hygiene pattern as PR #700 (which I just shipped + Mini caught me repeating). Empty env → skip the bump (no penalty). Set per-user. 2. Drop foundercoho from BLACKLIST_DOMAINS — overly specific brand. The generic noreply@/marketing@/newsletter@/no-reply@ patterns catch it (and arbitrary other brands) anyway. Added bare `noreply@` alongside `no-reply@` for completeness. 3. Cache TTL 15min → 30min. Materially improves cache-hit rate (~70% → ~95% per Mini's analysis) for minor freshness sacrifice; live gws fallback covers the freshness edge case anyway. 4. Replace brittle "DO NOT pass max=1" negative anti-pattern guidance with schema-level ignore + positive description: - tools.ts execute() now logs and ignores `max` when mode=important ("cache returns top-3 regardless") - Description: positive phrasing — "max is ignored in important mode; pass max only with mode='recent'". Harder for LLM to misread. - Parameter zod description follows the same shape. Smoke-tested all 3 paths: - mode=important default → cache hit, top-3 - mode=important + max=1 → logs "ignoring max=1", still returns top-3 - mode=recent + max=3 → live gws, count=3 All Mini's substantive nits addressed; minor style observations (O(N²) sort with N≤30, CACHE_PATH cwd assumption) left as documented in his review since they're non-broken under current usage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…hi#700) * fix(oss): remove Chi-specific paths/IDs from tool descriptions Two hardcoded literals that bake an OSS-user-failing assumption into the shipped code (caught by Chi 2026-05-14, both via OSS user's Discord drop): 1. `src/inline-tools.ts:30` — openFileTool description embedded `/Users/wangchi/.sutando-memory-sync/voice-contexts/ag2ai-investor.txt` as a literal example. OSS users have no `wangchi` home, no `.sutando-memory-sync` repo, no `ag2ai-investor` voice context — and the Gemini tool-call layer sees the example verbatim. Replaced with a `<active>` placeholder so the example stays self-documenting but doesn't leak a specific user's filesystem. 2. `skills/phone-conversation/scripts/conversation-server.ts:971` — phone-call summary task instructed the post-call agent to "Send a concise version (3-5 bullet points) to Discord DM (channel 1485370959870431433)" — Chi's specific DM channel. OSS users don't have that channel. Rewrote to point at the bridge's runtime resolution: `$SUTANDO_DM_OWNER_ID / access.json allowFrom`, which `src/dm-result.py` already implements correctly. Behavior changes: - Owner instance: no change (env vars + access.json resolve the same way). - OSS user: phone-call summaries now land in *their* owner DM channel, and the voice context example doesn't tell their Gemini to read a non-existent file. Typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(oss): simplify post-call summary DM instruction (Mini review nit) The previous instruction read two ways to the post-call agent: - Correct: "deliver via the standard task-bridge result-file path" - Confused: "you need to resolve $SUTANDO_DM_OWNER_ID / access.json yourself" The post-call agent doesn't have direct access to env vars or access.json — those are bridge-internal. Step 4 of the same instruction block already specifies the result-file write, and the bridge auto-resolves the DM channel from there. Drop the "(bridge resolves ...)" qualifier; rely on the implicit task-bridge contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…onichi#704) * feat(gws-gmail-voice): cache-first 3-tier path + importance scoring Solves two compounding bugs from PR sonichi#702 voice testing: 1) Gemini interpreted "most important email" → singular → passed max=1 2) Tool returned N-by-recency, no importance ranking → wrong top hit Architecture (Chi 2026-05-14): - Tier 1 CACHE — read `state/external-cache/inbox-important.json` (mtime <15min). Pre-scored top-3 importance-ranked. Sub-50ms. - Tier 2 LIVE — cache stale/missing or mode=recent → call gws (10s timeout). - Tier 3 WORK — gws fails → description tells Gemini to call `work` next. Cache writer: `skills/gws-gmail-voice/scripts/refresh-cache.py`. Reads `gws gmail +triage --max 30`, scores each via importance heuristics from feedback_triage_recency_not_importance.md: - BLACKLIST domains (substack, linkedin digest, FounderCoHo, ODSC, etc.): -100 - Self-notifs (Chi Wang via github.com notifications, CI failures): +15 - Academic .edu senders, openreview.net: +5/+10 - Subject patterns (deadline/RSVP/CI failure/accepted/urgent/review): +10 each - Calendar invitations: +8 - Sorts top-3 (score > -100), atomic-writes JSON. Tool description tweaks (the real fix to the max=1 bug): - Mode is now explicit: "important" (default) returns pre-ranked top-3; "recent" returns top-N by date. - Explicit instruction: "DO NOT pass max=1 — the tool already returns the most important; max only matters for mode=recent." - Pre-empts Gemini's previous misinterpretation. Run cache writer via ACT loop opportunistically OR ad-hoc: python3 skills/gws-gmail-voice/scripts/refresh-cache.py Smoke-tested end-to-end: - mode=important + fresh cache → top_3: CI failure / Xinyang Han .edu / GitHub PR - mode=recent + live gws → newest-first: Levay LinkedIn / CI / FounderCoHo - Cache stale path: falls through to live correctly. state/external-cache/ is gitignored (real email metadata); only the writer script + tool changes ship. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(gws-gmail-voice): apply Mini PR sonichi#704 review nits 1. @chiwang.cc → $SUTANDO_OWNER_EMAIL_DOMAIN env var. Same OSS-hygiene pattern as PR sonichi#700 (which I just shipped + Mini caught me repeating). Empty env → skip the bump (no penalty). Set per-user. 2. Drop foundercoho from BLACKLIST_DOMAINS — overly specific brand. The generic noreply@/marketing@/newsletter@/no-reply@ patterns catch it (and arbitrary other brands) anyway. Added bare `noreply@` alongside `no-reply@` for completeness. 3. Cache TTL 15min → 30min. Materially improves cache-hit rate (~70% → ~95% per Mini's analysis) for minor freshness sacrifice; live gws fallback covers the freshness edge case anyway. 4. Replace brittle "DO NOT pass max=1" negative anti-pattern guidance with schema-level ignore + positive description: - tools.ts execute() now logs and ignores `max` when mode=important ("cache returns top-3 regardless") - Description: positive phrasing — "max is ignored in important mode; pass max only with mode='recent'". Harder for LLM to misread. - Parameter zod description follows the same shape. Smoke-tested all 3 paths: - mode=important default → cache hit, top-3 - mode=important + max=1 → logs "ignoring max=1", still returns top-3 - mode=recent + max=3 → live gws, count=3 All Mini's substantive nits addressed; minor style observations (O(N²) sort with N≤30, CACHE_PATH cwd assumption) left as documented in his review since they're non-broken under current usage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Two hardcoded literals that bake an OSS-user-failing assumption into the shipped code, caught after an OSS user's Discord drop:
src/inline-tools.ts:30—openFileTooldescription had/Users/wangchi/.sutando-memory-sync/voice-contexts/ag2ai-investor.txtas a literal example. Replaced with<active>placeholder.skills/phone-conversation/scripts/conversation-server.ts:971— phone-summary task instructed delivery to hardcoded channel1485370959870431433(Chi's DM). Rewrote to reference the bridge's runtime resolution ($SUTANDO_DM_OWNER_ID/access.jsonallowFrom), whichsrc/dm-result.pyalready implements correctly.Behavior
Diff size
2 files, +2/-2. Typecheck clean (
npx tsc --noEmit).Not addressed in this PR
sonichi/sutandoGitHub URLs insrc/health-check.py,src/session-handoff.sh,src/web-client.ts, etc. — these are correct repo references for OSS users to find the canonical repo. Not bugs.VERIFIED_CALLERS, voice context names) are correctly env-gated or.env-driven; no action needed.🤖 Generated with Claude Code