feat: skill wiring, tool consolidation, and tool_lookup introspection#222
feat: skill wiring, tool consolidation, and tool_lookup introspection#222suryaiyer95 merged 1 commit intomainfrom
Conversation
Claude Code ReviewThis repository is configured for manual code reviews. Comment |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
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. |
There was a problem hiding this comment.
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
ToolRegistryand addtool_lookupto 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.
.opencode/skills/pii-audit/SKILL.md
Outdated
| For each query or dbt model, check which PII columns it accesses: | ||
|
|
||
| ``` | ||
| altimate_core_query_pii(sql: <sql>, schema: <schema_yaml>) |
There was a problem hiding this comment.
Fixed — updated to altimate_core_query_pii(sql: <sql>, schema_context: <schema_object>).
.opencode/skills/dbt-test/SKILL.md
Outdated
| Before running, validate the compiled model SQL to catch syntax and schema errors early: | ||
|
|
||
| ``` | ||
| altimate_core_validate(sql: <compiled_sql>, dialect: <dialect>) |
There was a problem hiding this comment.
Fixed — updated to altimate_core_validate(sql: <compiled_sql>, schema_context: <schema_object>). Removed non-existent dialect param.
Code reviewFound 1 issue:
altimate-code/packages/opencode/src/agent/agent.ts Lines 111 to 129 in 602a82e 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
f434d45 to
b93623d
Compare
- 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>
b93623d to
83bacb2
Compare
Summary
sql-review,schema-migration,pii-audit); rewritebuilder.txtwith tool-lookup guidancetool_lookupintrospection tool that reads live registryTools removed (duplicates of wrappers)
altimate_core_transpilesql_translatealtimate_core_formatsql_formataltimate_core_rewritesql_optimizealtimate_core_lintaltimate_core_checkaltimate_core_safetyaltimate_core_checkaltimate_core_is_safealtimate_core_checkaltimate_core_optimize_for_queryaltimate_core_prune_schematool_lookup— registry introspectionReads 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 typecheckpasses across all packages🤖 Generated with Claude Code