Skip to content

docs(skills): extract and correct the authoring skills from the closed PR #4 - #9

Open
0xSagaCity wants to merge 1 commit into
masterfrom
REF-24453-skills-extract
Open

docs(skills): extract and correct the authoring skills from the closed PR #4#9
0xSagaCity wants to merge 1 commit into
masterfrom
REF-24453-skills-extract

Conversation

@0xSagaCity

Copy link
Copy Markdown
Member

Replaces the skills half of the closed PR #4 ("Data Mapping Skill upgrade").

Why this exists

PR #4 changed 76 files. Eight were genuinely useful agent skills; the other 68 were a fork sync — new client templates (fitking, saga-engineering, mmd), widgets, tests, webpack.config.js, package-lock.json. It was CONFLICTING against master and has been closed. This branch takes the skills alone, and fixes the claims inside them that were wrong.

Targets master directly and does not stack on anything, so it can land independently of the validator PRs.

What lands

Three new skills:

Skill Question it answers
architect-template What must this block be able to render? — the completeness spec, before markup
ceres-template-data-contract Which contract field feeds this visible row? — path resolution, missing-data gate
data-mapping What does the built template actually do with this payload? — the audit, plus a field inventory for the whole document

Plus corrections to design-to-template, scaffold-template, snapshot-testing and navigate-codebase, and a new citation gate.

Corrections, each verified against refrens/ceres@master

The important one — the skills taught this fork's drifted field names. They said the inter-state and union-territory flags are invoice.igst / invoice.utgst. The canonical runtime reads invoice.isIgst / invoice.isUtgst (src/main/invoiceTemplateNormalization.ts lines 296, 399, 481, 516). This fork's own copy still reads the drifted names, which is the live defect scheduled for the epic's M0 — so shipping the skills as written would have turned a two-file code bug into documentation that propagates it into every future template. The skills now teach the canonical names.

Column keys, visibility flags and totals keys (columns[igst], showIgst, showCgstSgst, finalTotal.igst) are correct in both repos and were deliberately left alone — this was not a blanket rename.

The HSN summary had no business gate at all. The canonical runtime gates it on hsnSummaryEnabled (advanceOptions.showHsnSummary, alias showHSNSummaryInInvoice — the alias is checked first because it is what the Lydia bridge emits). No skill mentioned it, so an author following them renders the summary off a non-empty hsnList alone and leaks a section the business switched off. Added in two places.

Then the dangling references, all of which would have sent an agent looking for something that does not exist:

  • data-binding-tests was named 7 times. There is no such skill, here or on the source PR. The rule it carried — one test file per template, the mapping frozen as assertions inside it — is now stated directly.
  • tests/mmd.test.ts was cited twice as the pattern to copy. It belongs to a fork client template that is not in this repo. The pattern is three lines, so it is stated inline instead, including the guard that stops the check passing vacuously.
  • amountInWords — the path was right, the mechanism was not. src/widgets/shared/amountInWords.ts is a plain module; the {{amountInWords}} helper is registered only by src/widgets/tax-summary/index.ts:24 and src/widgets/hsn-summary/index.ts:24, two independent copies. Importing the shared module gives you a function and no helper.
  • helpers.ts was assumed to exist on every template. Neither template on master has one.
  • scaffold-template step 3 told you to run npm run test:snapshots:update — required, not conditional — while pointing at a note saying that script does not exist. Rewritten to ask for the verification without naming a command that isn't there.
  • navigate-codebase's Lydia citations were real but unprefixed, so they read as four missing local paths. Now cited lydia:src/....
  • The CSS section contradicted itself: it mandates --<prefix>-font-size-<step> and explicitly says not to abbreviate, then told you to lint for --*-fs-*. Also replaced the fork-template prefixes (sg/fk/mmd) with this repo's templates.

Deliberately not extracted

render-check. It documents npm run render, scripts/render.mjs, scripts/renderLint.mjs and .ceres-render/ artifacts. None of that exists on master — it arrives with the render-tooling extraction, which is worth doing as its own PR. Shipping a skill that tells you to run a script that isn't there is the exact defect this PR is cleaning up. snapshot-testing's reference to it was rewritten accordingly.

agents/openai.yaml — vendor assistant display config, not skill content.

The gate

.agent/skills/verify-skills.sh. A skill is documentation an agent acts on without checking, so every concrete claim has to resolve:

  1. Every unprefixed path exists in the tree.
  2. Every npm run <x> is defined in package.json.
  3. Every named sibling skill exists under .agent/skills/.
  4. No skill teaches a fork-drifted payload field name.
  5. No skill references the un-extracted render tooling.
  6. Frontmatter is well formed and name: matches its directory.

Two conventions make it workable. Paths are cited repo:path, so a cross-repo citation is explicit instead of indistinguishable from a broken one. And a skill documenting tooling that is not built yet declares itself with <!-- verify-skills: aspirational-scripts --> and is exempt from check 2 — that is how snapshot-testing keeps an honest "not wired up" banner without the gate going vacuous. The banner also moved to the top of that file, above the first command, so an agent reading top-down learns the harness is missing before it reads anything to run.

$ bash .agent/skills/verify-skills.sh
== 1. every local path resolves (repo-prefixed paths are cross-repo, skipped)
== 2. every npm script referenced is defined
== 3. every sibling skill named actually exists
== 4. canonical payload field names — fork drift must not be taught
== 5. un-extracted render tooling must not be referenced
== 6. frontmatter shape and name/directory agreement

OK: skills verified            exit 0

Mutation-tested rather than just run — each of four seeded defects turns it red, and reverting turns it green:

Seeded defect Gate output
isUtgstutgst FAIL … teaches a fork-drifted field name: 207:…
invoicePayloadContract.tsnope.ts FAIL … cites missing local path: src/main/nope.ts
`data-mapping``data-binding-tests` FAIL … names a nonexistent skill: data-binding-tests
npm run build:templatenpm run test:snapshots FAIL … tells you to run an undefined script: npm run test:snapshots

Exit codes confirmed: 0 clean, 1 dirty, 0 restored.

Follow-ups this does not do

  • Extract the render tooling (scripts/render.mjs, renderLint.mjs, dev-server.mjs, the render npm script, the .ceres-render/ gitignore entry) and re-land render-check on top of it.
  • Build the snapshot baseline harness, or delete snapshot-testing's command list. It is honest now, but it is still a manual for something that does not run.
  • Reconcile the skill duplication across repos. refrens/ceres and ceres-example carry byte-identical copies of debug-build, design-to-template, navigate-codebase, scaffold-template and snapshot-testing, with no sync — the same two-place problem as the contract. The three new skills land only here. Worth resolving when the toolkit is extracted.
  • Wire the gate into CI and the commit hook once the render tooling has settled.

refrens/ceres#26 adds a validate:templates step to that repo's copy of scaffold-template. It is not added here: the script does not exist in ceres-example on this branch. It arrives with PR #6.

…d PR #4

PR #4 ("Data Mapping Skill upgrade", closed) carried 8 genuinely useful skill
files inside 76 changed files, the other 68 being a fork sync: new client
templates, widgets, tests and webpack changes. This takes only the skills, and
corrects the claims that were wrong or dangling.

Adds three skills:
- architect-template — decide a block's full row set before writing markup, so
  a row the reference document happens not to show still renders when its data
  arrives.
- ceres-template-data-contract — resolve each visible row to a contract path,
  and stop the build when a row has no source.
- data-mapping — audit a real payload against a built template, one record per
  rendered element, plus a field inventory for the whole document.

Corrections applied, each verified against refrens/ceres@master:
- Taught the canonical payload flags isIgst / isUtgst. The skills had inherited
  this fork's drifted igst / utgst, which the real payload never sets;
  documenting that would have propagated a live rendering defect into every
  future template. Column keys, visibility flags and totals keys are correct in
  both repos and were left alone.
- Added the missing HSN-summary business gate hsnSummaryEnabled. Without it a
  template renders the summary off a non-empty hsnList alone and leaks a
  section the business switched off.
- Removed every reference to data-binding-tests, a skill that does not exist.
  The rule it carried, one test file per template with the mapping frozen as
  assertions inside it, is now stated directly.
- Replaced two pointers to tests/mmd.test.ts, a fork template's test, with the
  CSS-parse pattern inline, including the non-vacuity guard.
- Corrected the amountInWords mechanism: the module is shared, but the helper
  is registered only by the tax-summary and hsn-summary widgets, so importing
  the shared module alone gives you no helper.
- Marked helpers.ts optional; neither template on master has one.
- Rewrote scaffold step 3, which told you to run a snapshot script while
  pointing at a note saying that script does not exist.
- Prefixed navigate-codebase's Lydia citations with the repo name; they were
  real, but read as missing local paths.
- Replaced the fork-template CSS prefixes with this repo's, and made the
  font-size lint match the mandated spelling rather than the one the same
  skill forbids.

Deliberately not extracted: render-check. It documents a render script and its
artifacts, none of which exist here; it belongs with the separate render-tooling
extraction. The OpenAI assistant yaml was dropped as vendor harness config.

Adds .agent/skills/verify-skills.sh, a citation gate over the whole skill set:
local paths resolve, npm scripts are defined, named sibling skills exist,
payload fields use canonical names, and no skill references the un-extracted
render tooling. Paths are cited repo:path, so a cross-repo citation is explicit
rather than indistinguishable from a broken one. A skill documenting tooling
that is not built yet declares itself with a marker comment and is exempt from
the script check, which is how snapshot-testing keeps an honest not-wired-up
note without the gate going vacuous.

Verified: gate exits 0 clean, and exits 1 on each of four seeded defects, being
a drifted field name, a missing path, a dangling skill name and an undefined
script.
Sign up for free to 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.

1 participant