Skip to content

feat: skill wiring, tool consolidation, and tool_lookup introspection#222

Merged
suryaiyer95 merged 1 commit intomainfrom
worktree-skill-wiring
Mar 18, 2026
Merged

feat: skill wiring, tool consolidation, and tool_lookup introspection#222
suryaiyer95 merged 1 commit intomainfrom
worktree-skill-wiring

Conversation

@suryaiyer95
Copy link
Contributor

Summary

  • Phase 1 — Skill Wiring: Wire unwired tools into existing skills; add 3 new skills (sql-review, schema-migration, pii-audit); rewrite builder.txt with tool-lookup guidance
  • Phase 2 — Tool Consolidation: Remove 7 duplicate inner tools from the registry (saves ~1000 tokens/turn); add tool_lookup introspection tool that reads live registry
  • Comprehensive test suite: 51 tests across 3 files — unit tests, wire-format verification, LLM simulation (real Sonnet), and complex multi-tool workflow scenarios

Tools removed (duplicates of wrappers)

Removed Kept (wrapper)
altimate_core_transpile sql_translate
altimate_core_format sql_format
altimate_core_rewrite sql_optimize
altimate_core_lint altimate_core_check
altimate_core_safety altimate_core_check
altimate_core_is_safe altimate_core_check
altimate_core_optimize_for_query altimate_core_prune_schema

tool_lookup — registry introspection

Reads from the live tool registry. Returns tool description, parameters (name, type, required, description). Can never go stale.

Test plan

  • bun test tool-consolidation.test.ts — 30 deterministic tests (no LLM)
  • bun test tool-consolidation-e2e.test.ts — 21 tests (tier 1: wire-format, tier 1b: direct execution, tier 2+3: LLM simulation via subprocess)
  • turbo typecheck passes across all packages

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 17, 2026 19:27
@claude
Copy link

claude bot commented Mar 17, 2026

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

@github-actions
Copy link

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Consolidates the tool registry by removing duplicate AltimateCore tools, adds a tool_lookup introspection tool, and updates agent permissions/prompts/skills to align with the new tool surface—backed by deterministic and LLM-simulation test coverage.

Changes:

  • Remove 7 duplicate tool IDs from ToolRegistry and add tool_lookup to introspect the live registry.
  • Update restricted-agent permissions and the builder prompt to reference the consolidated tool set and tool_lookup.
  • Add extensive unit + E2E simulation tests and introduce new/updated skills (e.g., sql-review, schema-migration, pii-audit).

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
packages/opencode/src/tool/registry.ts Drops duplicate tool registrations and registers the new tool_lookup tool.
packages/opencode/src/altimate/tools/tool-lookup.ts Implements tool registry introspection and parameter/type formatting.
packages/opencode/src/altimate/prompts/builder.txt Updates builder guidance/tool references and adds tool_lookup guidance.
packages/opencode/src/agent/agent.ts Removes permissions for deleted tool IDs and allows tool_lookup for restricted agents.
packages/opencode/test/tool/tool-consolidation.test.ts Adds deterministic tests for registry changes, prompt/skill references, and tool_lookup behavior.
packages/opencode/test/tool/tool-consolidation-sim.ts Adds an LLM-driven simulation runner used by E2E tests.
packages/opencode/test/tool/tool-consolidation-e2e.test.ts Adds tiered E2E tests including subprocess LLM simulations and workflow scenarios.
.opencode/skills/sql-review/SKILL.md Adds a new SQL review skill describing a multi-tool quality gate workflow.
.opencode/skills/schema-migration/SKILL.md Adds a new schema migration analysis skill leveraging migration + schema diff tools.
.opencode/skills/query-optimize/SKILL.md Expands query optimization workflow to include explain plans and equivalence checks.
.opencode/skills/pii-audit/SKILL.md Adds a new PII audit skill combining schema classification and query exposure checks.
.opencode/skills/dbt-troubleshoot/SKILL.md Enhances troubleshooting workflow with quick-fix tools (altimate_core_fix, sql_fix).
.opencode/skills/dbt-test/SKILL.md Adds AltimateCore test generation and validation steps to the dbt testing workflow.
.opencode/skills/dbt-develop/SKILL.md Adds guidance for schema search and dialect awareness via profiles/validation/lineage tooling.
.opencode/skills/dbt-analyze/SKILL.md Improves lineage guidance (preferring dbt_lineage) and adds metadata extraction tooling.
.opencode/skills/cost-report/SKILL.md Expands FinOps workflow to include unused resource detection and query history enrichment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

For each query or dbt model, check which PII columns it accesses:

```
altimate_core_query_pii(sql: <sql>, schema: <schema_yaml>)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — updated to altimate_core_query_pii(sql: <sql>, schema_context: <schema_object>).

Before running, validate the compiled model SQL to catch syntax and schema errors early:

```
altimate_core_validate(sql: <compiled_sql>, dialect: <dialect>)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — updated to altimate_core_validate(sql: <compiled_sql>, schema_context: <schema_object>). Removed non-existent dialect param.

@suryaiyer95
Copy link
Contributor Author

Code review

Found 1 issue:

  1. altimate_core_rewrite was re-added to the tool registry in commit 602a82e but its permission was NOT added to any restricted agent's allowlist. The analyst, executive, validator, migrator, and researcher agents all use "*": "deny" as their base rule, which means altimate_core_rewrite is silently blocked in all restricted agent modes. Before this PR, the tool was explicitly allowed in these agents. This is a regression — the tool exists in the registry but is inaccessible from restricted agents.

PermissionNext.fromConfig({
"*": "deny",
sql_execute: "allow", sql_validate: "allow", sql_analyze: "allow",
sql_translate: "allow", sql_optimize: "allow", lineage_check: "allow",
warehouse_list: "allow", warehouse_test: "allow", warehouse_discover: "allow",
schema_inspect: "allow", schema_index: "allow", schema_search: "allow",
schema_cache_status: "allow", sql_explain: "allow", sql_format: "allow",
sql_fix: "allow", sql_autocomplete: "allow", sql_diff: "allow",
finops_query_history: "allow", finops_analyze_credits: "allow",
finops_expensive_queries: "allow", finops_warehouse_advice: "allow",
finops_unused_resources: "allow", finops_role_grants: "allow",
finops_role_hierarchy: "allow", finops_user_roles: "allow",
schema_detect_pii: "allow", schema_tags: "allow", schema_tags_list: "allow",
altimate_core_validate: "allow", altimate_core_check: "allow",
tool_lookup: "allow",
read: "allow", grep: "allow", glob: "allow",
question: "allow", webfetch: "allow", websearch: "allow",
training_save: "allow", training_list: "allow", training_remove: "allow",
}),

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@suryaiyer95 suryaiyer95 force-pushed the worktree-skill-wiring branch 2 times, most recently from f434d45 to b93623d Compare March 18, 2026 04:17
- Remove 6 duplicate tools from registry: `altimate_core_transpile`,
  `altimate_core_format`, `altimate_core_lint`, `altimate_core_safety`,
  `altimate_core_is_safe`, `altimate_core_optimize_for_query`
- Add `tool_lookup` introspection tool for runtime schema discovery
- Wire unwired tools into 9 SKILL.md files with correct param names
- Add `altimate_core_rewrite` and `tool_lookup` to all agent allowlists
- Add 3 new skills: `sql-review`, `schema-migration`, `pii-audit`

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@suryaiyer95 suryaiyer95 force-pushed the worktree-skill-wiring branch from b93623d to 83bacb2 Compare March 18, 2026 04:23
@suryaiyer95 suryaiyer95 merged commit d189794 into main Mar 18, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants