Skip to content

Add psl-ast-layers skill: choosing the right PSL syntax tree layer#911

Merged
SevInf merged 1 commit into
mainfrom
ast-skill
Jul 3, 2026
Merged

Add psl-ast-layers skill: choosing the right PSL syntax tree layer#911
SevInf merged 1 commit into
mainfrom
ast-skill

Conversation

@SevInf

@SevInf SevInf commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Linked issue

n/a — small change (no ticket; agent-skill documentation only).

At a glance

New skill at skills-contrib/psl-ast-layers/SKILL.md. The core of what it teaches, in one exchange:

// BAD: manual token hunt on a known node
let lbrace: SyntaxToken | undefined;
for (const child of model.syntax.children()) {
  if (child instanceof SyntaxToken && child.kind === 'LBrace') {
    lbrace = child;
    break;
  }
}

// GOOD: the getter already exists
const lbrace = model.lbrace();

Until now the layering rules for the PSL syntax tree (green tree vs red tree vs typed AST classes) lived only in reviewers' heads; agents working on PSL interpreters and the language server kept re-deriving them — or not.

Decision

Ship one agent skill, psl-ast-layers, that fires on any PSL-related work (contract-psl interpreters, psl-parser internals, language server, formatters) and encodes:

  1. The layer contract — green tree is storage foundation only (never touched by consumers); red tree is for navigation outside the current node; typed AST getters are the default for structural reads on a known node.
  2. Four anti-patterns with bad/good examples — re-stringifying the AST (printSyntax / SourceFile offset-slicing) to extract structural facts, reading through node.green, collecting lazy child iterators into arrays, and red-tree spelunking on nodes of known type.
  3. The current navigation-helper surfacefindAncestor, tokenAtOffset (+ seam-aware TokenAtOffset biasing), coveringElement, nextToken/prevToken, and the trivia-aware helpers from syntax/navigation.ts, so hand-rolled ancestor/whitespace-skipping loops are called out as re-implementations.

How it fits together

  1. A decision table maps each layer to its one job, then a numbered "Choosing a layer" flow walks from known node (typed getters) → outward/sideways movement (red-tree helpers, immediately re-entering the typed layer via static casts, combinable with any(…)) → unknown node type (offset anchoring via tokenAtOffset/coveringElement) → green tree (parser-internal only).
  2. A "missing getter" section directs authors to add getters to the AST classes in syntax/ast/ (built from findChildToken/findFirstChild/filterChildren) instead of hand-rolling child iteration at call sites.
  3. The anti-patterns section gives each violation a concrete bad/good pair using real APIs from the package.
  4. A quick-reference list closes with the full navigation and casting surface, including LSP position mapping (sourceFile.positionAt/offsetAt).

Reviewer notes

  • Every API named in the skill was verified against packages/1-framework/2-authoring/psl-parser/src/syntax/ at current origin/main — including the navigation helpers that landed in TML-2946: Add PSL type and generic block completions #871 (findAncestor, tokenAtOffset, SyntaxToken as a class, syntax/navigation.ts, any combinator).
  • printSyntax and SourceFile offsets are deliberately not banned outright — the skill carves out their legitimate human-facing uses (error snippets, formatter output, LSP ranges) and forbids only extracting structural facts from re-stringified text.
  • The lazy-iterator rule is stated absolutely (no "when arrays are OK" carve-out) — deliberate, to keep the instruction unambiguous for agents.
  • Per repo convention, the skill lives at its canonical home under skills-contrib/; the .agents/skills/ and .claude/skills/ symlinks are materialized by the prepare hook and stay untracked.

Testing performed

  • pnpm lint:skills — all skills-contrib skills pass frontmatter/structure validation.

Skill update

This PR is a skill addition (skills-contrib/psl-ast-layers/SKILL.md); no user-facing product surface changed, so no updates under packages/0-shared/skills/ are required.

Alternatives considered

  • A rule (.agents/rules/*.mdc) instead of a skill — rules are always-on and terse; this content needs worked examples and a decision flow, which fits the on-demand skill format. The layering guidance is only relevant when touching PSL code.
  • Documenting inside the psl-parser README — package docs describe the API; the skill encodes usage discipline (what not to do and why) targeted at agents, with fire conditions in the description. Both can coexist; the skill links to the package paths.
  • Listing legitimate array-materialization cases — an earlier draft outlined when Array.from on child iterators is acceptable; dropped to keep the anti-pattern absolute and easier for agents to follow.

Checklist

  • All commits are signed off (git commit -s) per the DCO. The DCO status check will block merge if any commit is missing a Signed-off-by: trailer.
  • I read CONTRIBUTING.md and the change is scoped to one logical concern.
  • Tests are updated (n/a — doc-only; pnpm lint:skills validates the skill).
  • The PR title is in TML-NNNN: <sentence-case title> form — intentionally omitted: no Linear ticket exists for this change (maintainer decision).
  • The Skill update section above is filled in.

Summary by CodeRabbit

  • Documentation
    • Added guidance on using the parser’s syntax tree layers correctly.
    • Clarified when to work with each layer and how to move between them safely.
    • Included recommended patterns for extracting structure and navigating nodes.
    • Listed common anti-patterns to avoid, plus a quick reference for parsing, casting, offsets, and positions.

New agent skill documenting the three PSL syntax tree layers (green tree,
red tree, strongly-typed AST classes), when to use each, and the four
anti-patterns to avoid (re-stringifying the AST, reading green nodes,
collecting lazy child iterators into arrays, and red-tree spelunking on
nodes of known type). Covers the navigation helpers from the red tree
(findAncestor, tokenAtOffset, coveringElement, trivia-aware movement).

Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
@SevInf SevInf requested a review from a team as a code owner July 3, 2026 15:18
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 28c1a39e-c1ee-4505-8979-8efab253cef3

📥 Commits

Reviewing files that changed from the base of the PR and between fc1eb0f and dbc0c20.

📒 Files selected for processing (1)
  • skills-contrib/psl-ast-layers/SKILL.md

📝 Walkthrough

Walkthrough

Adds a new documentation file, SKILL.md, under skills-contrib/psl-ast-layers, describing the PSL parser's three syntax tree layers (green tree, red tree, typed AST), when to use each, recommended getter-based navigation patterns, anti-patterns to avoid, and a quick-reference API list.

Changes

PSL AST Layers Documentation

Layer / File(s) Summary
Documentation page for AST layers
skills-contrib/psl-ast-layers/SKILL.md
New Markdown file documenting the green/red/typed AST layers, layer-selection guidance, recommended navigation patterns, anti-patterns, and quick-reference APIs.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Related Issues: None specified

Related PRs: None specified

Suggested labels: documentation

Suggested reviewers: None specified

Poem: A rabbit hops through layers three, / Green, red, and typed as can be, / With getters clean and paths made clear, / No text round-trips, no spelunking here, / A tidy doc for all to see! 🐇📄

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the new psl-ast-layers skill and its focus on selecting the right PSL syntax tree layer.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ast-skill

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@SevInf SevInf merged commit a2791c5 into main Jul 3, 2026
20 checks passed
@SevInf SevInf deleted the ast-skill branch July 3, 2026 15:30
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