Skip to content

fix(skills): use single-line quoted YAML for skill description - #10

Merged
krokoko merged 2 commits into
awslabs:mainfrom
scoropeza:fix/skill-description-parsing
Feb 6, 2026
Merged

fix(skills): use single-line quoted YAML for skill description#10
krokoko merged 2 commits into
awslabs:mainfrom
scoropeza:fix/skill-description-parsing

Conversation

@scoropeza

Copy link
Copy Markdown
Contributor

Summary

Fix SKILL.md description parsing bug where Claude Code displays the plugin name instead of the actual skill description.

Problem

Claude Code's skill parser doesn't correctly handle YAML multi-line block scalars (>). When the description uses this format, parsing fails silently and Claude falls back to displaying the plugin name.

Before (broken):

description: > Deploy applications to AWS. Triggers on: "deploy to AWS", "host on AWS"...

Shows: Deploy infrastructure to AWS (from plugin:deploy-on-aws@awslabs-agent-plugins).

After (fixed):

description: "Deploy applications to AWS. Triggers on: deploy to AWS, host on AWS..."

Shows the full description text.

Why Quotes Are Needed

The description contains colons (e.g., Triggers on:). Without quotes, YAML interprets these as key-value separators, causing a parsing error.

Test Plan

  1. Add marketplace: /plugin marketplace add scoropeza/agent-plugins --ref fix/skill-description-parsing
  2. Install plugin: /plugin install deploy-on-aws@awslabs-agent-plugins
  3. Ask Claude: "List your skills with their names and descriptions"
  4. Verify the deploy skill shows the full description, not just the plugin name

Screenshots

BeforeAfter
Generic plugin name as descriptionFull description text displayed

Your Name added 2 commits February 5, 2026 23:08
Multi-line block scalars (>) were not being parsed correctly by Claude Code's
skill parser, causing the description to show as the plugin name instead of
the actual description text.
@scoropeza
scoropeza requested a review from a teamFebruary 6, 2026 07:26
@krokoko
krokoko self-requested a review February 6, 2026 14:05
@krokoko
krokoko enabled auto-merge February 6, 2026 14:05
@krokoko
krokoko added this pull request to the merge queueFeb 6, 2026
Merged via the queue into awslabs:main with commit 0762c5eFeb 6, 2026
1 check passed
@krokoko
krokoko deleted the fix/skill-description-parsing branch February 6, 2026 14:40

@scottschreckengaustscottschreckengaust left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

amaksimo added a commit that referenced this pull request May 8, 2026
Correctness:
- #3: Restore 3k-row cap in Quick Start step 3
- #6: Add PostgreSQL parser caveat to Workflow 7 (MySQL syntax → parse error → fallback)
- #10: Fix ORM pattern to present fixed_with_warning to user (not auto-accept)
- #12: Unfixable rewrites MUST present to user before substituting
Docs accuracy:
- #7: Rails: use db:structure:dump with schema_format = :sql
- #8: Prisma: add required --from-empty --to-schema-datamodel flags
Error handling:
- #16: Add Error Handling section for dsql_lint failures (MCP unavailable, parse error, timeout)
Evals:
- #5: Add evals 102/103 to results summary table
- #9: Fix model metadata (remove specific version)
- #11: Add missing category_id column to eval 103 prompt
Already fixed in previous commit:
- #17: dsql-lint removed from description (93e9c7b)
PR body items (1, 2, 4) will be updated separately.
amaksimo added a commit that referenced this pull request May 8, 2026
Correctness:
- #3: Restore 3k-row cap in Quick Start step 3
- #6: Add PostgreSQL parser caveat to Workflow 7 (MySQL syntax → parse error → fallback)
- #10: Fix ORM pattern to present fixed_with_warning to user (not auto-accept)
- #12: Unfixable rewrites MUST present to user before substituting
Docs accuracy:
- #7: Rails: use db:schema:dump with schema_format = :sql (6.1+)
- #8: Prisma: add required --from-empty --to-schema-datamodel flags
- SQLAlchemy: use CreateTable(table).compile(engine) instead of metadata.create_all(echo=True) which executes DDL
Error handling:
- #16: Add Error Handling section for dsql_lint failures (MCP unavailable, parse error, timeout) with user-confirmation gates
- Add dsql_lint-unavailable entry to SKILL.md Error Scenarios
Evals:
- #5: Add evals 102/103 to results MD with detail sections
- #9: Fix model metadata (remove specific version; clarify manual grading)
- #11: Add missing category_id column to eval 103 prompt
- Tighten eval expectations to reference concrete tool outputs (rule names, summary fields)
- Replace emoji markers with PASS/FAIL/PARTIAL to fix dprint table alignment
- Bump recorded dsql-lint version to 0.1.4
Self-review fixes (17 sub-agent review rounds):
- Document accurate fix_result.status enum: fixed | fixed_with_warning | unfixable
(tool emits status='unfixable' explicitly; earlier doc incorrectly implied absence)
- Scope Unfixable Errors table to truly-unfixable rules only (set_transaction, truncate,
create_table_as, add_column_constraint, index_expression, index_partial,
unsupported_alter_table_op); note that temp_table, inherits, index_using,
transaction_isolation are fixed/fixed_with_warning
- Fix transaction_isolation vs set_transaction rule-id confusion
- Promote reference-load gate from SHOULD to MUST with tightened trigger
- Workflow 2: explicit lint gate for async index DDL (step 5)
- Workflow 6: lint every generated DDL in Table Recreation Pattern
- Workflow 7: cross-check MySQL source against type-mapping.md even on clean lint
(ENGINE=, SET() pass silently through PostgreSQL parser)
- Document 1M-char SQL limit and 30s server timeout
- Require user confirmation before destructive DDL (DROP/RENAME/TRUNCATE), MCP-unavailable
fallback, parse_error manual rewrite, and timeout split-retry paths
- Forbid executing fixed_sql while any unfixable diagnostic remains (re-lint until clean)
- Add user override semantics for "just run it" requests
- Remove redundant Usage Patterns, Exit Codes, and Additional Resources sections
Already fixed in previous commit:
- #17: dsql-lint removed from description (93e9c7b)
PR body items (1, 2, 4) will be updated separately.
amaksimo added a commit that referenced this pull request May 8, 2026
Correctness:
- #3: Restore 3k-row cap in Quick Start step 3
- #6: Add PostgreSQL parser caveat to Workflow 7 (MySQL syntax → parse error → fallback)
- #10: Fix ORM pattern to present fixed_with_warning to user (not auto-accept)
- #12: Unfixable rewrites MUST present to user before substituting
Docs accuracy:
- #7: Rails: use db:schema:dump with schema_format = :sql (6.1+)
- #8: Prisma: add required --from-empty --to-schema-datamodel flags
- SQLAlchemy: use CreateTable(table).compile(engine) instead of metadata.create_all(echo=True) which executes DDL
Error handling:
- #16: Add Error Handling section for dsql_lint failures (MCP unavailable, parse error, timeout) with user-confirmation gates
- Add dsql_lint-unavailable entry to SKILL.md Error Scenarios
Evals:
- #5: Add evals 102/103 to results MD with detail sections
- #9: Fix model metadata (remove specific version; clarify manual grading)
- #11: Add missing category_id column to eval 103 prompt
- Tighten eval expectations to reference concrete tool outputs (rule names, summary fields)
- Replace emoji markers with PASS/FAIL/PARTIAL to fix dprint table alignment
- Bump recorded dsql-lint version to 0.1.4
Self-review fixes (17 sub-agent review rounds):
- Document accurate fix_result.status enum: fixed | fixed_with_warning | unfixable
(tool emits status='unfixable' explicitly; earlier doc incorrectly implied absence)
- Scope Unfixable Errors table to truly-unfixable rules only (set_transaction, truncate,
create_table_as, add_column_constraint, index_expression, index_partial,
unsupported_alter_table_op); note that temp_table, inherits, index_using,
transaction_isolation are fixed/fixed_with_warning
- Fix transaction_isolation vs set_transaction rule-id confusion
- Promote reference-load gate from SHOULD to MUST with tightened trigger
- Workflow 2: explicit lint gate for async index DDL (step 5)
- Workflow 6: lint every generated DDL in Table Recreation Pattern
- Workflow 7: cross-check MySQL source against type-mapping.md even on clean lint
(ENGINE=, SET() pass silently through PostgreSQL parser)
- Document 1M-char SQL limit and 30s server timeout
- Require user confirmation before destructive DDL (DROP/RENAME/TRUNCATE), MCP-unavailable
fallback, parse_error manual rewrite, and timeout split-retry paths
- Forbid executing fixed_sql while any unfixable diagnostic remains (re-lint until clean)
- Add user override semantics for "just run it" requests
- Remove redundant Usage Patterns, Exit Codes, and Additional Resources sections
Already fixed in previous commit:
- #17: dsql-lint removed from description (93e9c7b)
PR body items (1, 2, 4) will be updated separately.
amaksimo added a commit that referenced this pull request May 8, 2026
Correctness:
- #3: Restore 3k-row cap in Quick Start step 3
- #6: Add PostgreSQL parser caveat to Workflow 7 (MySQL syntax → parse error → fallback)
- #10: Fix ORM pattern to present fixed_with_warning to user (not auto-accept)
- #12: Unfixable rewrites MUST present to user before substituting
Docs accuracy:
- #7: Rails: use db:schema:dump with schema_format = :sql (6.1+)
- #8: Prisma: add required --from-empty --to-schema-datamodel flags
- SQLAlchemy: use CreateTable(table).compile(engine) instead of metadata.create_all(echo=True) which executes DDL
Error handling:
- #16: Add Error Handling section for dsql_lint failures (MCP unavailable, parse error, timeout) with user-confirmation gates
- Add dsql_lint-unavailable entry to SKILL.md Error Scenarios
Evals:
- #5: Add evals 102/103 to results MD with detail sections
- #9: Fix model metadata (remove specific version; clarify manual grading)
- #11: Add missing category_id column to eval 103 prompt
- Tighten eval expectations to reference concrete tool outputs (rule names, summary fields)
- Replace emoji markers with PASS/FAIL/PARTIAL to fix dprint table alignment
- Bump recorded dsql-lint version to 0.1.4
Self-review fixes (17 sub-agent review rounds):
- Document accurate fix_result.status enum: fixed | fixed_with_warning | unfixable
(tool emits status='unfixable' explicitly; earlier doc incorrectly implied absence)
- Scope Unfixable Errors table to truly-unfixable rules only (set_transaction, truncate,
create_table_as, add_column_constraint, index_expression, index_partial,
unsupported_alter_table_op); note that temp_table, inherits, index_using,
transaction_isolation are fixed/fixed_with_warning
- Fix transaction_isolation vs set_transaction rule-id confusion
- Promote reference-load gate from SHOULD to MUST with tightened trigger
- Workflow 2: explicit lint gate for async index DDL (step 5)
- Workflow 6: lint every generated DDL in Table Recreation Pattern
- Workflow 7: cross-check MySQL source against type-mapping.md even on clean lint
(ENGINE=, SET() pass silently through PostgreSQL parser)
- Document 1M-char SQL limit and 30s server timeout
- Require user confirmation before destructive DDL (DROP/RENAME/TRUNCATE), MCP-unavailable
fallback, parse_error manual rewrite, and timeout split-retry paths
- Forbid executing fixed_sql while any unfixable diagnostic remains (re-lint until clean)
- Add user override semantics for "just run it" requests
- Remove redundant Usage Patterns, Exit Codes, and Additional Resources sections
Already fixed in previous commit:
- #17: dsql-lint removed from description (93e9c7b)
PR body items (1, 2, 4) will be updated separately.
Morlej pushed a commit to Morlej/agent-plugins that referenced this pull request May 8, 2026
…dation (awslabs#157)
* feat(dsql): add dsql_lint tool integration for SQL compatibility validation
Add dsql-lint as a deterministic validation tool the agent invokes before
executing externally-sourced SQL. Enables migration support for customers
coming from PostgreSQL, MySQL, or ORMs (Django, Rails, Prisma, TypeORM).
Changes:
- Add references/dsql-lint.md: tool API, fix statuses, usage patterns,
ORM integration, unfixable error resolution
- Update SKILL.md: add dsql_lint to MCP Tools section, update Workflows
2/6/7 with lint validation steps, add Workflow 9 (Validate & Migrate
SQL to DSQL)
- Update frontmatter: add lint/ORM trigger phrases and tags
The dsql_lint MCP tool (shipping separately in awslabs/mcp) validates SQL
and optionally auto-fixes issues, returning structured diagnostics the
agent acts on. The skill teaches the agent when and how to use it.
* fix: resolve markdownlint errors (MD029, MD032)
- MD029: Restart ordered list numbering after section breaks
- MD032: Add blank lines before lists after bold headings
* refactor: trim SKILL.md to 276 lines (under 300 limit)
Move AWS Knowledge limits table reference to development-guide.md (already
documented there). Condense Quick Start to 3 lines. Trim workflow
descriptions to routing-only — detail lives in reference files.
validate-size.py: 276 lines, status 'good'
validate-references.py: 0 broken links, 0 new orphans
* fix: address review feedback
- Restore destructive workflow warning in Workflow 6
- Re-introduce RFC language (MUST, MAY) in Quick Start
- Use active voice: 'Use get_schema', 'Use transact', 'Use readonly_query'
- Restore 'one DDL per transaction, multiple DML may share' framing
- Remove 'lint' from tags (not sufficient alone to trigger skill)
- Remove TOC from dsql-lint.md (file is short, TOC adds no value)
* style: apply dprint table formatting to dsql-lint.md
Run dprint fmt to align table columns per repo formatting rules.
* feat: add dsql_lint eval harness and tool availability fallback
- Add tools/evals/databases-on-aws/dsql/dsql_lint_evals.json with 4
functional evals covering: pg_dump migration, Django ORM migration,
clean SQL validation, and MySQL with unfixable issues
- Add availability note to dsql-lint.md: fall back to manual validation
using existing DDL rules when the MCP tool is not yet available
The evals test that the agent calls dsql_lint before executing SQL,
presents warnings to the user, and handles unfixable errors correctly.
* fix: remove incorrect availability fallback note
The dsql_lint tool and the skill that references it will ship together
in the same MCP repo PR. There is no availability gap — the fallback
note was based on a wrong assumption about PR splitting.
* feat: add eval harness results from local dsql_lint testing
Run dsql_lint_evals.json against local MCP server with dsql_lint tool.
All 4 evals pass — tool correctly identifies compatibility issues,
produces fixed SQL, and reports unfixable errors for manual resolution.
Key findings:
- Eval 103 (MySQL syntax): dsql-lint uses a PostgreSQL parser, so
MySQL-specific syntax (SET, ENGINE, PARTITION BY) triggers a parse
error rather than individual rules. Agent falls back to
mysql-migrations reference for these cases.
* feat: replace tool-only eval results with behavioral with-skill vs baseline comparison
Run evals as subagent behavioral tests: one agent with the skill loaded
(uses dsql_lint), one baseline without (relies on model knowledge).
Key findings:
- Baseline hallucinates JSON→JSONB (DSQL rejects JSONB as column type)
- Baseline misses CREATE INDEX ASYNC requirement
- Baseline doesn't split multi-DDL transactions
- Skill-guided agent uses dsql_lint for deterministic validation,
produces correct output on all three failure points
The iron law holds: the agent fails without this skill change.
* fix: address review feedback round 2
- Restore hardcoded limits table (critical for performance, not all
are in dev guide, link to DSQL docs prevents stale numbers)
- Merge Workflow 9 into Workflow 7 as 'Validate and Migrate to DSQL'
(reduces line count, single entry point for all migration sources)
- Trim redundant triggers from description (lint SQL covers dsql-lint,
migrate to DSQL covers ORM migration DSQL)
290 lines, mise run build passes.
* fix: remove dsql-lint from description, trim trigger
Per review: 'SQL compatibility validation' is sufficient without
naming the tool. Remove 'via dsql-lint' and 'lint SQL for DSQL'
trigger — 'migrate to DSQL' already covers the use case.
* fix: address code review findings (18-item tracker)
Correctness:
- awslabs#3: Restore 3k-row cap in Quick Start step 3
- awslabs#6: Add PostgreSQL parser caveat to Workflow 7 (MySQL syntax → parse error → fallback)
- awslabs#10: Fix ORM pattern to present fixed_with_warning to user (not auto-accept)
- awslabs#12: Unfixable rewrites MUST present to user before substituting
Docs accuracy:
- awslabs#7: Rails: use db:schema:dump with schema_format = :sql (6.1+)
- awslabs#8: Prisma: add required --from-empty --to-schema-datamodel flags
- SQLAlchemy: use CreateTable(table).compile(engine) instead of metadata.create_all(echo=True) which executes DDL
Error handling:
- awslabs#16: Add Error Handling section for dsql_lint failures (MCP unavailable, parse error, timeout) with user-confirmation gates
- Add dsql_lint-unavailable entry to SKILL.md Error Scenarios
Evals:
- awslabs#5: Add evals 102/103 to results MD with detail sections
- awslabs#9: Fix model metadata (remove specific version; clarify manual grading)
- awslabs#11: Add missing category_id column to eval 103 prompt
- Tighten eval expectations to reference concrete tool outputs (rule names, summary fields)
- Replace emoji markers with PASS/FAIL/PARTIAL to fix dprint table alignment
- Bump recorded dsql-lint version to 0.1.4
Self-review fixes (17 sub-agent review rounds):
- Document accurate fix_result.status enum: fixed | fixed_with_warning | unfixable
(tool emits status='unfixable' explicitly; earlier doc incorrectly implied absence)
- Scope Unfixable Errors table to truly-unfixable rules only (set_transaction, truncate,
create_table_as, add_column_constraint, index_expression, index_partial,
unsupported_alter_table_op); note that temp_table, inherits, index_using,
transaction_isolation are fixed/fixed_with_warning
- Fix transaction_isolation vs set_transaction rule-id confusion
- Promote reference-load gate from SHOULD to MUST with tightened trigger
- Workflow 2: explicit lint gate for async index DDL (step 5)
- Workflow 6: lint every generated DDL in Table Recreation Pattern
- Workflow 7: cross-check MySQL source against type-mapping.md even on clean lint
(ENGINE=, SET() pass silently through PostgreSQL parser)
- Document 1M-char SQL limit and 30s server timeout
- Require user confirmation before destructive DDL (DROP/RENAME/TRUNCATE), MCP-unavailable
fallback, parse_error manual rewrite, and timeout split-retry paths
- Forbid executing fixed_sql while any unfixable diagnostic remains (re-lint until clean)
- Add user override semantics for "just run it" requests
- Remove redundant Usage Patterns, Exit Codes, and Additional Resources sections
Already fixed in previous commit:
- awslabs#17: dsql-lint removed from description (93e9c7b)
PR body items (1, 2, 4) will be updated separately.
Morlej added a commit to Morlej/agent-plugins that referenced this pull request May 15, 2026
…vals
Correctness fixes (review items 1-5):
- awslabs#1: push-computation-to-constant — use NUMERIC column 'amount' to
avoid integer division non-equivalence
- awslabs#2: not-in-to-not-exists — add NULL semantics warning (NOT EXISTS
does not preserve NOT IN's NULL-propagation; MUST confirm with user)
- awslabs#3/awslabs#4: subquery-unnesting — prefer EXISTS form (true semi-join);
document uniqueness precondition for JOIN+DISTINCT alternative
- awslabs#5: subquery-unnesting-scalar — add COALESCE(s_count, 0) for
COUNT/SUM (LEFT JOIN returns NULL, scalar returns 0)
Dangling reference fixes (review items 6-8):
- awslabs#6: workflow.md trigger table — "Phase 5" → reassessment re-entry
- awslabs#7: Replace all "implicit cast compatibility matrix" references
with "pg_amop query in catalog-queries.md"
- awslabs#8: plan-interpretation.md L202 — fix cast-vs-operator contradiction
Structural fixes (review items 9-14, 24):
- awslabs#9: Hedge "integer family" claim with "at time of writing" + verify
- awslabs#10: amopmethod=10003 — add provenance comment and verification SQL
- awslabs#11: catalog-queries.md TOC — add 3 missing sections
- awslabs#12: plan-interpretation.md TOC — add Type Coercion section
- awslabs#13: SKILL.md — explicitly delegate routing to workflow.md
- awslabs#24: workflow.md — remove em dashes from headings for clean anchors
Other fixes (review items 21-23):
- awslabs#21: reltuples-estimate — add staleness warning (MUST warn user)
- awslabs#22: catalog-queries — add safe_query.build() note for placeholders
- awslabs#23: "Skip when" → "SHOULD skip when" in all rewrite files
Eval improvements (review items 14, 16):
- awslabs#14: README — add query_plan_rewrite_evals to directory tree and
eval section
- awslabs#16: Add evals 206-210 covering LEFT JOIN, computation push, NOT IN
with NULL warning, nested UNION ALL, and negative case (OR across
different columns)
- awslabs#7 (eval): Update eval 201 expectation — pg_amop instead of matrix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
anwesham-lab pushed a commit to Morlej/agent-plugins that referenced this pull request May 26, 2026
…ML parse error (awslabs#175)
The model-evaluation SKILL.md description is an unquoted YAML plain scalar that
contains a colon-space sequence ("two evaluation types: LLM-as-Judge"). The YAML
parser reads that colon as a mapping indicator and rejects the frontmatter with
"mapping values are not allowed here". When this happens the skill loads with
empty metadata (name, description, and version are silently dropped), so the
skill no longer triggers and "claude plugin validate" reports an error.
Wrapping the description in single quotes makes it a valid quoted scalar while
preserving the text verbatim, including the inner double quotes and the colon.
This is the same class of fix as awslabs#10.
Co-authored-by: pyt <youtae@commoncoding.io>
anwesham-lab pushed a commit to Morlej/agent-plugins that referenced this pull request May 26, 2026
…vals
Correctness fixes (review items 1-5):
- awslabs#1: push-computation-to-constant — use NUMERIC column 'amount' to
avoid integer division non-equivalence
- awslabs#2: not-in-to-not-exists — add NULL semantics warning (NOT EXISTS
does not preserve NOT IN's NULL-propagation; MUST confirm with user)
- awslabs#3/awslabs#4: subquery-unnesting — prefer EXISTS form (true semi-join);
document uniqueness precondition for JOIN+DISTINCT alternative
- awslabs#5: subquery-unnesting-scalar — add COALESCE(s_count, 0) for
COUNT/SUM (LEFT JOIN returns NULL, scalar returns 0)
Dangling reference fixes (review items 6-8):
- awslabs#6: workflow.md trigger table — "Phase 5" → reassessment re-entry
- awslabs#7: Replace all "implicit cast compatibility matrix" references
with "pg_amop query in catalog-queries.md"
- awslabs#8: plan-interpretation.md L202 — fix cast-vs-operator contradiction
Structural fixes (review items 9-14, 24):
- awslabs#9: Hedge "integer family" claim with "at time of writing" + verify
- awslabs#10: amopmethod=10003 — add provenance comment and verification SQL
- awslabs#11: catalog-queries.md TOC — add 3 missing sections
- awslabs#12: plan-interpretation.md TOC — add Type Coercion section
- awslabs#13: SKILL.md — explicitly delegate routing to workflow.md
- awslabs#24: workflow.md — remove em dashes from headings for clean anchors
Other fixes (review items 21-23):
- awslabs#21: reltuples-estimate — add staleness warning (MUST warn user)
- awslabs#22: catalog-queries — add safe_query.build() note for placeholders
- awslabs#23: "Skip when" → "SHOULD skip when" in all rewrite files
Eval improvements (review items 14, 16):
- awslabs#14: README — add query_plan_rewrite_evals to directory tree and
eval section
- awslabs#16: Add evals 206-210 covering LEFT JOIN, computation push, NOT IN
with NULL warning, nested UNION ALL, and negative case (OR across
different columns)
- awslabs#7 (eval): Update eval 201 expectation — pg_amop instead of matrix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
amaksimo added a commit that referenced this pull request May 28, 2026
- Add table of contents to data-loading.md (standard #10: >100 lines)
- Reframe version-specific language to be timeless (standard #5)
- Add routing intro to diagnostic decision tree (standard #20)
- Tighten Going Deeper cross-reference to avoid duplication (standard #9)
- Add data-loading trigger keywords to SKILL.md description (standard #1)
- Add data-loading tag to metadata
Morlej added a commit to Morlej/agent-plugins that referenced this pull request Jun 17, 2026
…vals
Correctness fixes (review items 1-5):
- awslabs#1: push-computation-to-constant — use NUMERIC column 'amount' to
avoid integer division non-equivalence
- awslabs#2: not-in-to-not-exists — add NULL semantics warning (NOT EXISTS
does not preserve NOT IN's NULL-propagation; MUST confirm with user)
- awslabs#3/awslabs#4: subquery-unnesting — prefer EXISTS form (true semi-join);
document uniqueness precondition for JOIN+DISTINCT alternative
- awslabs#5: subquery-unnesting-scalar — add COALESCE(s_count, 0) for
COUNT/SUM (LEFT JOIN returns NULL, scalar returns 0)
Dangling reference fixes (review items 6-8):
- awslabs#6: workflow.md trigger table — "Phase 5" → reassessment re-entry
- awslabs#7: Replace all "implicit cast compatibility matrix" references
with "pg_amop query in catalog-queries.md"
- awslabs#8: plan-interpretation.md L202 — fix cast-vs-operator contradiction
Structural fixes (review items 9-14, 24):
- awslabs#9: Hedge "integer family" claim with "at time of writing" + verify
- awslabs#10: amopmethod=10003 — add provenance comment and verification SQL
- awslabs#11: catalog-queries.md TOC — add 3 missing sections
- awslabs#12: plan-interpretation.md TOC — add Type Coercion section
- awslabs#13: SKILL.md — explicitly delegate routing to workflow.md
- awslabs#24: workflow.md — remove em dashes from headings for clean anchors
Other fixes (review items 21-23):
- awslabs#21: reltuples-estimate — add staleness warning (MUST warn user)
- awslabs#22: catalog-queries — add safe_query.build() note for placeholders
- awslabs#23: "Skip when" → "SHOULD skip when" in all rewrite files
Eval improvements (review items 14, 16):
- awslabs#14: README — add query_plan_rewrite_evals to directory tree and
eval section
- awslabs#16: Add evals 206-210 covering LEFT JOIN, computation push, NOT IN
with NULL warning, nested UNION ALL, and negative case (OR across
different columns)
- awslabs#7 (eval): Update eval 201 expectation — pg_amop instead of matrix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Morlej added a commit to Morlej/agent-plugins that referenced this pull request Jun 19, 2026
…vals
Correctness fixes (review items 1-5):
- awslabs#1: push-computation-to-constant — use NUMERIC column 'amount' to
avoid integer division non-equivalence
- awslabs#2: not-in-to-not-exists — add NULL semantics warning (NOT EXISTS
does not preserve NOT IN's NULL-propagation; MUST confirm with user)
- awslabs#3/awslabs#4: subquery-unnesting — prefer EXISTS form (true semi-join);
document uniqueness precondition for JOIN+DISTINCT alternative
- awslabs#5: subquery-unnesting-scalar — add COALESCE(s_count, 0) for
COUNT/SUM (LEFT JOIN returns NULL, scalar returns 0)
Dangling reference fixes (review items 6-8):
- awslabs#6: workflow.md trigger table — "Phase 5" → reassessment re-entry
- awslabs#7: Replace all "implicit cast compatibility matrix" references
with "pg_amop query in catalog-queries.md"
- awslabs#8: plan-interpretation.md L202 — fix cast-vs-operator contradiction
Structural fixes (review items 9-14, 24):
- awslabs#9: Hedge "integer family" claim with "at time of writing" + verify
- awslabs#10: amopmethod=10003 — add provenance comment and verification SQL
- awslabs#11: catalog-queries.md TOC — add 3 missing sections
- awslabs#12: plan-interpretation.md TOC — add Type Coercion section
- awslabs#13: SKILL.md — explicitly delegate routing to workflow.md
- awslabs#24: workflow.md — remove em dashes from headings for clean anchors
Other fixes (review items 21-23):
- awslabs#21: reltuples-estimate — add staleness warning (MUST warn user)
- awslabs#22: catalog-queries — add safe_query.build() note for placeholders
- awslabs#23: "Skip when" → "SHOULD skip when" in all rewrite files
Eval improvements (review items 14, 16):
- awslabs#14: README — add query_plan_rewrite_evals to directory tree and
eval section
- awslabs#16: Add evals 206-210 covering LEFT JOIN, computation push, NOT IN
with NULL warning, nested UNION ALL, and negative case (OR across
different columns)
- awslabs#7 (eval): Update eval 201 expectation — pg_amop instead of matrix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
anwesham-lab pushed a commit to Morlej/agent-plugins that referenced this pull request Jun 22, 2026
…vals
Correctness fixes (review items 1-5):
- awslabs#1: push-computation-to-constant — use NUMERIC column 'amount' to
avoid integer division non-equivalence
- awslabs#2: not-in-to-not-exists — add NULL semantics warning (NOT EXISTS
does not preserve NOT IN's NULL-propagation; MUST confirm with user)
- awslabs#3/awslabs#4: subquery-unnesting — prefer EXISTS form (true semi-join);
document uniqueness precondition for JOIN+DISTINCT alternative
- awslabs#5: subquery-unnesting-scalar — add COALESCE(s_count, 0) for
COUNT/SUM (LEFT JOIN returns NULL, scalar returns 0)
Dangling reference fixes (review items 6-8):
- awslabs#6: workflow.md trigger table — "Phase 5" → reassessment re-entry
- awslabs#7: Replace all "implicit cast compatibility matrix" references
with "pg_amop query in catalog-queries.md"
- awslabs#8: plan-interpretation.md L202 — fix cast-vs-operator contradiction
Structural fixes (review items 9-14, 24):
- awslabs#9: Hedge "integer family" claim with "at time of writing" + verify
- awslabs#10: amopmethod=10003 — add provenance comment and verification SQL
- awslabs#11: catalog-queries.md TOC — add 3 missing sections
- awslabs#12: plan-interpretation.md TOC — add Type Coercion section
- awslabs#13: SKILL.md — explicitly delegate routing to workflow.md
- awslabs#24: workflow.md — remove em dashes from headings for clean anchors
Other fixes (review items 21-23):
- awslabs#21: reltuples-estimate — add staleness warning (MUST warn user)
- awslabs#22: catalog-queries — add safe_query.build() note for placeholders
- awslabs#23: "Skip when" → "SHOULD skip when" in all rewrite files
Eval improvements (review items 14, 16):
- awslabs#14: README — add query_plan_rewrite_evals to directory tree and
eval section
- awslabs#16: Add evals 206-210 covering LEFT JOIN, computation push, NOT IN
with NULL warning, nested UNION ALL, and negative case (OR across
different columns)
- awslabs#7 (eval): Update eval 201 expectation — pg_amop instead of matrix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Morlej added a commit to Morlej/agent-plugins that referenced this pull request Jun 24, 2026
…vals
Correctness fixes (review items 1-5):
- awslabs#1: push-computation-to-constant — use NUMERIC column 'amount' to
avoid integer division non-equivalence
- awslabs#2: not-in-to-not-exists — add NULL semantics warning (NOT EXISTS
does not preserve NOT IN's NULL-propagation; MUST confirm with user)
- awslabs#3/awslabs#4: subquery-unnesting — prefer EXISTS form (true semi-join);
document uniqueness precondition for JOIN+DISTINCT alternative
- awslabs#5: subquery-unnesting-scalar — add COALESCE(s_count, 0) for
COUNT/SUM (LEFT JOIN returns NULL, scalar returns 0)
Dangling reference fixes (review items 6-8):
- awslabs#6: workflow.md trigger table — "Phase 5" → reassessment re-entry
- awslabs#7: Replace all "implicit cast compatibility matrix" references
with "pg_amop query in catalog-queries.md"
- awslabs#8: plan-interpretation.md L202 — fix cast-vs-operator contradiction
Structural fixes (review items 9-14, 24):
- awslabs#9: Hedge "integer family" claim with "at time of writing" + verify
- awslabs#10: amopmethod=10003 — add provenance comment and verification SQL
- awslabs#11: catalog-queries.md TOC — add 3 missing sections
- awslabs#12: plan-interpretation.md TOC — add Type Coercion section
- awslabs#13: SKILL.md — explicitly delegate routing to workflow.md
- awslabs#24: workflow.md — remove em dashes from headings for clean anchors
Other fixes (review items 21-23):
- awslabs#21: reltuples-estimate — add staleness warning (MUST warn user)
- awslabs#22: catalog-queries — add safe_query.build() note for placeholders
- awslabs#23: "Skip when" → "SHOULD skip when" in all rewrite files
Eval improvements (review items 14, 16):
- awslabs#14: README — add query_plan_rewrite_evals to directory tree and
eval section
- awslabs#16: Add evals 206-210 covering LEFT JOIN, computation push, NOT IN
with NULL warning, nested UNION ALL, and negative case (OR across
different columns)
- awslabs#7 (eval): Update eval 201 expectation — pg_amop instead of matrix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sbbhimji pushed a commit to sbbhimji/agent-plugins that referenced this pull request Jun 30, 2026
…ction, rewrites, and workflow extraction (awslabs#162)
* feat(dsql): enhance query plan explainability with type coercion detection and rewrite references
- Add structured trigger phrases and routing criteria for query plan diagnosis
- Add type coercion index bypass detection (implicit cast compatibility matrix)
- Extend catalog queries with indexed column type retrieval
- Add generic SQL rewrite reference (11 patterns: OR-to-IN, subquery unnesting, etc.)
- Add DSQL-specific rewrite reference (reltuples estimate, split large joins for DP threshold)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(dsql): extract query plan workflow, add rewrite evals
- Extract Workflow 8 (query plan explainability) from SKILL.md into
references/query-plan/workflow.md to stay under the 300 LOC limit
- Wire query-rewrites-generic.md and query-rewrites-dsql-specific.md
into the workflow (Phase 0 load list + Phase 2 evidence gathering)
- Add behavioral evals (query_plan_rewrite_evals.json) covering type
coercion detection, subquery unnesting, OR-to-IN, GROUP BY pushdown,
large join splitting, and reltuples estimation
- Add eval results (query_plan_rewrite_eval_results.md) with
with-skill vs baseline comparison
Validation:
- validate-size.py: 275 lines (good)
- validate-references.py: 0 broken links, 0 new orphans
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: correct markdown link fragments in workflow.md TOC
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* style: apply dprint table formatting
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* refactor(dsql): address PR review — split rewrites, positive language, RFC keywords
Review feedback from amaksimo:
- Split query-rewrites-generic.md into 11 individual files under
query-rewrites/ subdirectory to reduce context consumption
- Split query-rewrites-dsql-specific.md into individual files
- Convert monolithic files to index tables pointing to sub-files
- Fix DATEADD() SQL Server syntax → PostgreSQL NOW() - INTERVAL
- Flip negative language ("Do not apply") to positive ("Skip when")
- Add RFC keywords (MUST, SHOULD, MAY) throughout
- Remove psql fallback from workflow.md (enforce MCP usage)
- Update plan-interpretation.md recommendation template with RFC language
- Make Phase 0 loading explicit: MUST for core refs, SHOULD for rewrites
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(dsql): address PR awslabs#162 review — correctness, references, evals
Correctness fixes (review items 1-5):
- #1: push-computation-to-constant — use NUMERIC column 'amount' to
avoid integer division non-equivalence
- #2: not-in-to-not-exists — add NULL semantics warning (NOT EXISTS
does not preserve NOT IN's NULL-propagation; MUST confirm with user)
- awslabs#3/awslabs#4: subquery-unnesting — prefer EXISTS form (true semi-join);
document uniqueness precondition for JOIN+DISTINCT alternative
- awslabs#5: subquery-unnesting-scalar — add COALESCE(s_count, 0) for
COUNT/SUM (LEFT JOIN returns NULL, scalar returns 0)
Dangling reference fixes (review items 6-8):
- awslabs#6: workflow.md trigger table — "Phase 5" → reassessment re-entry
- awslabs#7: Replace all "implicit cast compatibility matrix" references
with "pg_amop query in catalog-queries.md"
- awslabs#8: plan-interpretation.md L202 — fix cast-vs-operator contradiction
Structural fixes (review items 9-14, 24):
- awslabs#9: Hedge "integer family" claim with "at time of writing" + verify
- awslabs#10: amopmethod=10003 — add provenance comment and verification SQL
- awslabs#11: catalog-queries.md TOC — add 3 missing sections
- awslabs#12: plan-interpretation.md TOC — add Type Coercion section
- awslabs#13: SKILL.md — explicitly delegate routing to workflow.md
- awslabs#24: workflow.md — remove em dashes from headings for clean anchors
Other fixes (review items 21-23):
- awslabs#21: reltuples-estimate — add staleness warning (MUST warn user)
- awslabs#22: catalog-queries — add safe_query.build() note for placeholders
- awslabs#23: "Skip when" → "SHOULD skip when" in all rewrite files
Eval improvements (review items 14, 16):
- awslabs#14: README — add query_plan_rewrite_evals to directory tree and
eval section
- awslabs#16: Add evals 206-210 covering LEFT JOIN, computation push, NOT IN
with NULL warning, nested UNION ALL, and negative case (OR across
different columns)
- awslabs#7 (eval): Update eval 201 expectation — pg_amop instead of matrix
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(dsql): address remaining PR awslabs#162 review items
- awslabs#17: Downgrade eval results to qualitative comparison, record model
and version, note n=1 and recommend n>=3 for production confidence
- awslabs#18: SKILL.md is 281 lines (will update PR body)
- awslabs#20: Strengthen awsknowledge fallback to MUST — refuse fallback when
recommendation depends on exact limit value
- awslabs#21: Already addressed in prior commit (reltuples staleness)
- awslabs#15: Document manual-only status and future Python converter direction
(per anwesham-lab's suggestion for deterministic rewrites)
- awslabs#19: MCP mirror PR noted as follow-up in PR body
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(dsql): address self-review findings — links, inclusive language, evals
- Revert broken relative links in SKILL.md (mcp/.mcp.json, scripts/)
back to correct ../../ paths
- Rename blacklisted_customers → excluded_customers and blacklist →
exclusion_list for inclusive language compliance
- Fix stale 'implicit cast compatibility matrix' → pg_amop in eval
results
- Add eval results for 206–210 (LEFT JOIN, computation push, NOT IN
NULL warning, nested UNION ALL, negative OR case)
- Include hooks.json update
* fix: remove stray conflict marker in evals README
* style: apply dprint table formatting to eval results
* fix(dsql): wire workflow.md and rewrite indexes into SKILL.md
Addresses amaksimo's review comment: after rebase onto main, the
Workflow 9 section lost its link to workflow.md and the rewrite index
files, making all 16 new query-plan files unreachable orphans.
- Add workflow.md as the entry gate in the reference table
- Add query-rewrites-generic.md and query-rewrites-dsql-specific.md
- Update Workflow 9 section to load workflow.md instead of listing
the 4 Phase-0 files directly (workflow.md handles that routing)
* fix(dsql): address review findings awslabs#4, awslabs#5, awslabs#7, awslabs#8, awslabs#9
- awslabs#4: COALESCE rule in subquery-unnesting-scalar.md restricted to
COUNT only; SUM/MAX/MIN return NULL on empty sets in both forms
- awslabs#5: verify-comment in catalog-queries.md changed from
amname='btree' to amname='btree_index' (DSQL uses btree_index AM)
- awslabs#7: workflow.md context disambiguation removes psql fallback offer,
now says no MCP means no plan capture (consistent with Safety)
- awslabs#8: split-large-joins.md example expanded from 7 to 11 tables,
exceeding the stated DP threshold of 10; CTEs project explicit cols
- awslabs#9: plan-interpretation.md recommendation template changed ::float
to ::integer (only integer-family cross-type operators registered)
* fix(dsql): correct safe_query substitution guidance in catalog-queries
Lift the substitution rule to the file preamble with per-position
guidance: identifier positions (FROM, GROUP BY) use ident(); string-
literal positions (WHERE = '{schema}', IN ('{table}')) use allow()
or regex(). The prior note incorrectly prescribed ident() for all
positions, which would produce invalid SQL in WHERE clauses.
* refactor(dsql): merge in-subquery-to-exists into subquery-unnesting-uncorrelated
Delete redundant in-subquery-to-exists.md — same input shape and same
rewrite as subquery-unnesting-uncorrelated.md. Merge the 'large result
set' trigger and 'small static set' skip condition into the canonical
file. Update index and eval 200 to route exclusively there.
* style: apply dprint table formatting to workflow.md
* fix(dsql): address merge-blocking review findings #1-awslabs#4
- #1: Strip surrounding quotes from all placeholders in catalog-queries
so safe_query helpers (which emit their own quotes) don't double-quote.
Add worked safe_query.build() example to preamble.
- #2: DP threshold changed from 10 to 8 (validated: SHOW
join_collapse_limit = 8 on live DSQL). Agent now instructed to SHOW
the value rather than hardcoding.
- awslabs#3: Remove pg_stat_user_tables.last_analyze cross-check (DSQL never
populates it). Guard reltuples with GREATEST(..., 0) for the -1
sentinel on never-analyzed tables.
- awslabs#4: Fix '11 generic' to '10 generic' in SKILL.md reference table.
* feat(dsql): add blog learnings — filter model, DPU, CTE rewrite
- Add Three-Layer Filter Model (Index Cond / Storage Filter / Query
Processor Filter) with optimization table to plan-interpretation.md
- Add Fixing Storage Lookups guidance (INCLUDE columns) with example
- Add Cost Number Interpretation (startup ~100 is normal in DSQL)
- Add DPU Interpretation (Read DPU as primary signal, optimization loop)
- Add CTE late materialization as DSQL-specific rewrite pattern
(defer Storage Lookups past LIMIT)
- Update workflow.md Phase 1: recommend plain EXPLAIN first for
expensive queries before EXPLAIN ANALYZE VERBOSE
* chore: bump databases-on-aws plugin version to 1.4.0
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for freeto 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.

4 participants

@scoropeza@scottschreckengaust@theagenticguy@krokoko