chore(ls): add skill for reviewing apidom-ls changes against spec#5183
Open
robert-hebel-sb wants to merge 4 commits into
Open
chore(ls): add skill for reviewing apidom-ls changes against spec#5183robert-hebel-sb wants to merge 4 commits into
robert-hebel-sb wants to merge 4 commits into
Conversation
Adds a Claude Code skill that automates spec-compliance review of apidom-ls configuration for any specification namespace. The skill encodes lessons from the A2A 1.0 review (PROVCON-5343): curl+Python fallback for long spec pages, deprecated-flow detection, systematic scopes gap checks, anchor format rules per spec site, and a structured report template with spec citations for every finding. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add coverage for type lint rules, completion file checks, discriminated union subtype registration, deprecated object detection, and enum/value-constraint rules — all gaps identified from the A2A 1.0 review comments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lessons Remove all A2A-specific references (examples, section headings, Key Lessons attribution) and generalize to apply to any spec namespace. Add improvements derived from PR #5110 review: new-field coverage when extending a parent version, version-scoped allowed-fields completeness, and array-of-objects linter function pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… skill Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
glowcloud
reviewed
Jul 6, 2026
|
|
||
| **Key pitfalls:** | ||
| - Deprecated or removed objects may appear only as field types within a parent, with **no separate section and no field table** — do not invent required/optional status for them | ||
| - Discriminated unions list their variant fields as `Optional (OneOf)` — this is distinct from "optional field on the object" |
Contributor
There was a problem hiding this comment.
This seems very A2A specific 🤔
| The spec page may truncate in `WebFetch`. Use the curl+Python approach above, searching for each section heading. Always take the **last** occurrence of a heading string (the actual content, not the TOC entry). | ||
|
|
||
| **Mandatory table verification — do not skip:** | ||
| After extracting a section, confirm you actually have the Fixed Fields table and not just the TOC entry. The extracted text MUST contain both the field name AND a `Yes` or `No` value in proximity. Use this pattern: |
Contributor
There was a problem hiding this comment.
Do we know if the skill will be fine in case where required/optional is in the description, e.g. in OAS, in comparison to A2A where it's a separate column?
| - A field in the array is not in the spec (extra field — causes false positives) | ||
| - A spec field is absent from the array (missing field — allows unknown fields through) | ||
|
|
||
| **When extending a parent version**: If a version-scoped file exists (e.g., `allowed-fields-3-2.ts`), verify it contains ALL fields valid for that version — both new fields and fields carried over from the parent. New fields added to an existing object are the most common source of omissions. Also check: if the 3.2 list is identical to the 3.1 list, the file may be redundant or the new fields may have been forgotten. |
Contributor
There was a problem hiding this comment.
Should we mention somewhere that for the existing rules we might need to verify that new targetSpec was added? And that new rules have targetSpec only for the new version?
| Spec pages typically have a table of contents near the top listing section headings (e.g. `4.4.1. AgentCard`). These TOC entries appear earlier in the document than the actual object definitions and contain **no** field data. If you extract a section and the result does not contain `Yes` or `No` in a `Required` column, you have the TOC entry, not the actual table — fetch again using the last-occurrence strategy. Never infer a field's required status from a TOC entry, a secondary source (proto file, JSON Schema), or prior knowledge. Always quote the raw table row as evidence before reporting a finding. | ||
|
|
||
| ### Deprecated Objects May Have No Spec Section | ||
| Objects removed or deprecated in a spec version often appear only as variant fields inside a parent (e.g. `Optional (OneOf)`), with no separate section and no field table. Any `--required.ts` rule for fields on such objects cannot be verified against the spec and should be removed or flagged. |
Contributor
There was a problem hiding this comment.
I feel like this is also A2A specific 😅 Mentioning that if there's no actual definition for the object and it is referenced in the parent, then there should be no required rule, might be fine 🤔
| A field that is an array of spec objects should use `apilintChildrenOfElementsOrClasses` (with the element class name), not `apilintType`. Using `apilintType` with `['array']` validates the container but not the element type of its members. | ||
|
|
||
| ### Deprecated/Removed Objects May Still Have Config Dirs | ||
| If a spec removes an object in a new version, its config directory may persist in the implementation. Always check the spec changelog and flag config dirs for objects that no longer exist in the target version. |
Contributor
There was a problem hiding this comment.
Let's maybe note that we should not remove that config if we support the old versions, it should just have the right targetSpec
| The spec page may truncate in `WebFetch`. Use the curl+Python approach above, searching for each section heading. Always take the **last** occurrence of a heading string (the actual content, not the TOC entry). | ||
|
|
||
| **Mandatory table verification — do not skip:** | ||
| After extracting a section, confirm you actually have the Fixed Fields table and not just the TOC entry. The extracted text MUST contain both the field name AND a `Yes` or `No` value in proximity. Use this pattern: |
Contributor
There was a problem hiding this comment.
Just a note that this skill probably can't be used to review JSON Schema - the rules are spread between a few specifications and they have everything in the field descriptions (at least for the existing ones), e.g. https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-01#section-6.1.1c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/skills/review-ls-changes/SKILL.md— a new Claude Code skill invoked via/review-ls-changes.claude/skills/SKILL-SUMMARY.mdwith an entry for the new skillThe skill performs structured spec-compliance reviews of
apidom-lsconfiguration for any specification namespace. It checks:config.ts)The skill was developed from the A2A 1.0 implementation review (PROVCON-5343) and encodes the lessons learned:
scopes--required.tsgap pattern across OAuth flows/specification/uses numbered anchors;/definitions/uses kebab-case)Test plan
/review-ls-changesin a Claude Code session and confirm it prompts for spec URL, config dir, and namespace packagehttps://a2a-protocol.org/latest/specification/packages/apidom-ls/src/config/a2a/and confirm report structure matches the template🤖 Generated with Claude Code