fix(sl-toolkit): edits use PATCH, and sql_dialect is snake_case - #94
Conversation
Two bugs, both verified against a live metastore (canary-orion). Either one is enough to
make the skill fail or do damage.
**1. `sqlDialect` is rejected — `/sl-build` cannot create a model today.**
The API requires snake_case `sql_dialect`; camelCase returns
`422 missing property 'sql_dialect'`. The skill's POST envelope, sl-build's model-create
call, and the test fixture all used `sqlDialect`, so the greenfield wizard fails at its
first write.
Both CI guards were asserting the wrong direction — `test_sqldialect_is_camelcase` and
`test_no_dialect_drift` required camelCase and *forbade* the correct key, and
`schemas/semantic-model.json` did the same in its `not.anyOf`. That is why the bug shipped
and stayed. Assertions inverted, and the consistency check now matches the quoted payload
key rather than the bare word so the gotchas section can still name `sqlDialect` in prose.
Also corrected the schema enum: only `Snowflake` and `BigQuery` are accepted. `Redshift`
and `Postgres` were listed but both return
`422 value must be one of 'Snowflake', 'BigQuery'`.
**2. `PATCH` exists — the documented DELETE + POST edit path is destructive.**
`PATCH /api/v1/repository/{type}/{id}` updates in place, keeps the object's UUID and bumps
its revision (verified 1 -> 2 on both a metric and a glossary term). The skill said "the
metastore has no PATCH" and built rollback machinery around delete-then-post, which mints a
new UUID, resets revision history, breaks anything referencing the old UUID, and can leave
the layer missing an object if the POST fails.
Rewrote the edit section around `api_patch` (new helper) — send only changed fields. The
rename cascade to constraint `metrics[]` is preserved and also now in-place. The rollback
block is gone: nothing is deleted, so there is nothing to roll back.
All 30 tests pass; confirmed the corrected guards fail when the bug is reintroduced.
|
CI note: Tier 0 (static lint + case validation) passes — that's the job covering these changes, and it's the one that was previously asserting the wrong casing. Tier 1 (skill activation routing) fails for an unrelated infra reason — the repo's Anthropic key is out of credit: It errors before evaluating any case, so it's not a signal about this diff. Locally: |
|
Re-checked the Tier 1 failure after the CI notification — it is not caused by this PR, and nothing in the diff can fix it. Evidence:
So: needs credit topped up on the repo's Anthropic account to go green. Happy to re-run once that's sorted. |
Resolve conflict from PR #94 (sl-toolkit: PATCH edits + snake_case sql_dialect). PR #94 modified 7 files under plugins/sl-toolkit/ which #88 consolidated into plugins/keboola/. Ported all content changes to the new locations: - commands/sl-build.md, skills/semantic-layer/SKILL.md, tests/{fixtures,schemas}/semantic-model.json, tests/test_skill_consistency.py, tests/test_smoke.py plugins/sl-toolkit/README.md: kept #88's deletion (per-plugin READMEs were consolidated); its one-line PATCH note is preserved in the ported SKILL.md.
Two bugs in the
semantic-layerskill, both verified against a live metastore(canary-orion, project 431). Either one alone is enough to make the skill fail or do damage.
Found while porting this skill into Kai's in-platform tooling
(AI-3661) —
rather than working around them downstream, fixing them here.
1.
sqlDialectis rejected —/sl-buildcannot create a model todayThe API requires snake_case
sql_dialect. camelCase gives:The skill's POST envelope,
sl-build's model-create call, and the test fixture all usedsqlDialect, so the greenfield wizard fails at its very first write.Both CI guards were asserting the wrong direction.
test_sqldialect_is_camelcaseandtest_no_dialect_driftrequired camelCase and forbade the correct key, andschemas/semantic-model.jsondid the same in itsnot.anyOf. That's why the bug shipped andstayed green. Inverted them, and the consistency check now matches the quoted payload key
instead of the bare word, so the gotchas section can still name
sqlDialectin prose to sayit's rejected.
Also corrected the schema enum — only
SnowflakeandBigQueryare accepted.RedshiftandPostgreswere listed; both return422 value must be one of 'Snowflake', 'BigQuery'.2.
PATCHexists — the documented DELETE + POST edit path is destructiveThe skill stated "The metastore has no PATCH — editing is DELETE old + POST updated" and built
rollback machinery around it.
PATCH /api/v1/repository/{type}/{id}works:019fc788-d6cb-7582-95d5-46d96f1bad9fVerified on both a
semantic-metricand asemantic-glossaryobject, so it isn't type-specific.Delete-then-post instead mints a new UUID, resets revision history, breaks anything
referencing the old UUID, and leaves the layer missing an object if the POST fails.
Rewrote the edit section around a new
api_patchhelper — send only the fields that change. Therename cascade to constraint
metrics[]is preserved and is now also in-place. The rollback blockis deleted: nothing is removed, so there is nothing to roll back.
Testing
All 30 tests pass. I also re-introduced the
sqlDialectbug in the fixture and confirmed thecorrected guards now catch it (2 failures) rather than demanding it.
Release Notes
Fixes the
semantic-layerskill's model-create payload (sql_dialect, notsqlDialect), whichwas rejected by the metastore with a 422, and replaces the destructive DELETE + POST edit
recipe with in-place
PATCHso object UUIDs and revision history survive an edit.