Skip to content

feat: kind-scoped tag types + tag builder functions (issues #10, #11)#14

Open
alltheseas wants to merge 12 commits into
mainfrom
feat/kind-scoped-tag-types
Open

feat: kind-scoped tag types + tag builder functions (issues #10, #11)#14
alltheseas wants to merge 12 commits into
mainfrom
feat/kind-scoped-tag-types

Conversation

@alltheseas

@alltheseas alltheseas commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Kind-scoped tag types (--kind-tags, issue #10)

  • New emit-kind-tags.ts emitter generates per-kind tag tuple types from KindShape[] — 312 types across 177 kinds (e.g., Kind30166_NTag, Kind10002_RTag)
  • Wire --kind-tags <file> CLI flag (included in --all as kind-tags.d.ts)
  • Handle edge cases: skip entries with insufficient positional data, disambiguate case-colliding tag letters (pPTag, PUpper_PTag)

Tag builder functions (--builders, issue #11)

  • New planner (plan-builders.ts) produces language-independent BuilderAction[] IR from KindShape
  • New emitter (emit-builders.ts) renders TypeScript interfaces + functions + dispatch
  • CLI: --builders <file> flag (included in --all)
  • 133 builder functions generated, covering all constrained kinds
  • anyOf groups emit runtime checks ("At least one of ... is required")
  • Constant-only tags (e.g., ["z", "order"]) emitted unconditionally without interface fields
  • Implied positions synthesized when minItems > positions.length

Generated API example

// Kind 38383 (NIP-69 order) — constant-only z tag emitted automatically
buildKind38383Tags({ d: "abc", k: "sell", s: "pending", ... })
// → [["d","abc"], ["k","sell"], ..., ["z","order"]]

// Kind 777 (Spell) — anyOf group enforced at runtime
buildKind777Tags({ cmd: "REQ", k: "1" })
// → [["cmd","REQ"], ["k","1"]]

buildKind777Tags({ cmd: "REQ" })
// → throws "At least one of k, authors, ... is required"

Closes #10, closes #11

Test plan

  • 393 tests passing (all new: kind-tag types + builder planner/emitter/round-trip)
  • Round-trip validation: builder output cross-checked against validator plan for all 133 constrained kinds
  • Generated builders.ts and kind-tags.d.ts compile with tsc --strict --noEmit
  • All pre-existing tests unbroken

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • CLI options to emit kind-scoped tag type declarations and builder code; --all emits both and logs generated counts.
    • Generates per-kind exported tag type aliases, per-kind builder functions, a dispatch builder and a constant of supported kinds.
  • Tests

    • Added comprehensive test suites validating tag/type emission, builder planning/generation, round-trip validation, and strict compilation checks.
  • Chores

    • Added Apache and MIT license files and updated package license field.

alltheseas and others added 4 commits March 28, 2026 00:24
Export emitPositionType/emitTupleType from emit-typescript.ts and add
new emit-kind-tags.ts that generates per-kind tag tuple types (e.g.,
Kind30166_NTag) from KindShape tag constraints. Collects tags from all
4 sources (requiredTags, perItemConditionals, arrayLevelConditionals,
anyOfTagGroups) with dedup preferring more constrained versions.

Includes 24 tests covering all exported functions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add kindTagsOut to CliArgs, --kind-tags flag to parseArgs, include in
--all defaults (kind-tags.d.ts), needKinds check, help text, and
output writing section.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Skip TagRequirements where minItems > positions.length (e.g., kind 777
"authors" has minItems:2 but only 1 extracted position), which
previously produced empty type aliases that failed tsc.

Prefix single uppercase tag letters with "Upper_" in type names so
case-distinct tags like "p" and "P" produce Kind1619_PTag vs
Kind1619_Upper_PTag instead of duplicate exports.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify --all writes kind-tags.d.ts, the output compiles under tsc
--strict, produces >= 300 types, contains no duplicate type names,
and has no empty type aliases (missing RHS).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Mar 28, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 82e93b4a-aaa9-427e-9cf5-f68cf6ced922

📥 Commits

Reviewing files that changed from the base of the PR and between d954dbb and efdc639.

📒 Files selected for processing (3)
  • LICENSE-APACHE
  • LICENSE-MIT
  • package.json
✅ Files skipped from review due to trivial changes (3)
  • LICENSE-APACHE
  • LICENSE-MIT
  • package.json

📝 Walkthrough

Walkthrough

Adds a planner and two emitters to generate per-kind tag TypeScript types and builder functions, integrates CLI flags to emit those artifacts, exports two TypeScript helper functions, and adds comprehensive unit and integration tests plus license files.

Changes

Cohort / File(s) Summary
Kind-scoped tag emitter
src/emit-kind-tags.ts
New emitter converting KindShape[]kind-tags.d.ts with tag-name → type-part mapping, per-kind tag collection/deduplication, and per-tag export type alias emission.
Builder planner
src/plan-builders.ts
New planner producing language‑independent BuilderAction[], exports FieldInputType/BuilderTag/BuilderAction and tagNameToFieldName, plus planBuilder(shape).
Builder emitter
src/emit-builders.ts
New emitter producing builders.ts: Kind{N}TagsInput interfaces, buildKind{N}Tags functions, numeric dispatch buildKindTags, and BUILDER_KINDS.
Type emission helpers exported
src/emit-typescript.ts
Made emitPositionType and emitTupleType exported for reuse (no behavior change).
CLI integration & orchestration
src/index.ts
Added --kind-tags and --builders CLI flags, updated --all defaults, adjusted kind extraction gating, and wired writes/logging for emitted artifacts.
Kind/tag tests
tests/emit-kind-tags.test.ts, tests/compile-check.test.ts
Unit tests for tag-name/type helpers, collection/deduplication, emission cases; compile-time strict tsc check and file-level assertions (counts, duplicates, empty aliases).
Builder planner & emitter tests
tests/plan-builders.test.ts, tests/emit-builders.test.ts
Tests for planner outputs (positions, input types, field-name derivation) and emitted builder code (interfaces, guards, anyOf handling, ordering).
Round-trip integration tests
tests/round-trip.test.ts
Simulates builder execution from planner actions, validates required/anyOf semantics, and optionally cross-checks against real schema artifacts when available.
Licenses & package metadata
LICENSE-APACHE, LICENSE-MIT, package.json
Added Apache and MIT license files and set license field to "(MIT OR Apache-2.0)".

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI Args
    participant Index as index.ts
    participant Planner as plan-builders.ts
    participant KindEmitter as emit-kind-tags.ts
    participant BuilderEmitter as emit-builders.ts
    participant TSHelpers as emit-typescript.ts
    participant FS as File System

    CLI->>Index: parse args (--kind-tags / --builders / --all)
    Index->>Index: decide needKinds
    Index->>Planner: collectKindTags / planBuilder(shapes)
    Planner-->>Index: KindTag entries / BuilderAction[]
    Index->>KindEmitter: emitKindTagsFile(shapes)
    KindEmitter->>TSHelpers: emitPositionType / emitTupleType
    TSHelpers-->>KindEmitter: type fragments
    KindEmitter-->>Index: kind-tags.d.ts content
    Index->>FS: write kind-tags.d.ts
    Index->>BuilderEmitter: emitBuildersFile(shapes)
    BuilderEmitter->>Planner: use BuilderAction[] / helpers
    BuilderEmitter-->>Index: builders.ts content
    Index->>FS: write builders.ts
    FS-->>Index: write results
    Index->>CLI: log counts
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰
I nibble names into tidy parts,
Kind-scoped types and builder arts.
From schema seeds the code now springs,
Tags and builders — oh how I sing! ✨

🚥 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 accurately describes the primary changes: kind-scoped tag types and tag builder functions, with explicit issue references.
Linked Issues check ✅ Passed The PR fully addresses both linked issues: #10 generates kind-scoped tag types (Kind30166_NTag, etc.) with --kind-tags CLI flag and emitKindTagsFile, and #11 generates builder functions per kind (buildKind30166Tags) with --builders CLI flag, planBuilder, and emitBuildersFile utilities.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the linked issues: kind-tag emitters, builders, CLI integration, tests, and license files. No unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/kind-scoped-tag-types

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/emit-kind-tags.ts (1)

135-144: Optional: guard minItems = 0 in the optional trailing case to avoid emitting empty tuple variants.

The loop at line 137 will generate variants starting from len = minItems. While minItems = 0 is theoretically possible in a schema, extraction logic consistently defaults minItems to either the number of defined positions or 2, so this is unlikely in practice. However, if it occurs, the resulting readonly [] type would be semantically incorrect for Nostr tags. Consider using Math.max(req.minItems, 1) as the loop start to ensure tags always require at least the tag name.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/emit-kind-tags.ts` around lines 135 - 144, The loop that builds tuple
variants currently starts at req.minItems and can emit an empty tuple when
req.minItems === 0; change the loop to start at Math.max(req.minItems, 1) (use
this when iterating len up to effectiveMax) to ensure the emitted tuples always
include at least one element (the tag name) and update the return guard around
variants (used to return `export type ${name} = ${variants[0]};`) to assume
variants[0] is present after this change; locate and modify the for loop that
references req.minItems/effectiveMax and the immediate check using
variants.length to apply this fix, keeping emitTupleType and name unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/emit-kind-tags.ts`:
- Around line 135-144: The loop that builds tuple variants currently starts at
req.minItems and can emit an empty tuple when req.minItems === 0; change the
loop to start at Math.max(req.minItems, 1) (use this when iterating len up to
effectiveMax) to ensure the emitted tuples always include at least one element
(the tag name) and update the return guard around variants (used to return
`export type ${name} = ${variants[0]};`) to assume variants[0] is present after
this change; locate and modify the for loop that references
req.minItems/effectiveMax and the immediate check using variants.length to apply
this fix, keeping emitTupleType and name unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 22cc72e3-ac82-4216-a4a8-a7b56bee8f19

📥 Commits

Reviewing files that changed from the base of the PR and between 9284323 and 60da16e.

📒 Files selected for processing (5)
  • src/emit-kind-tags.ts
  • src/emit-typescript.ts
  • src/index.ts
  • tests/compile-check.test.ts
  • tests/emit-kind-tags.test.ts

alltheseas and others added 4 commits March 28, 2026 01:42
New plan layer for tag construction, mirroring the existing
plan-validators.ts pattern. Transforms KindShape into language-
independent BuilderAction[] using collectKindTags() for tag dedup.

- BuilderPosition, FieldInputType, BuilderTag, BuilderAction types
- planBuilder() classifies tags as required/optional by source
- tagNameToFieldName() converts tag names to camelCase
- Sub-field name dedup for multi-position tags with colliding titles
- 17 unit tests covering all action types and edge cases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Generates typed builder functions from BuilderAction[] planner output.
Per kind: interface + function. File-level: dispatch + BUILDER_KINDS.

- Unique variable names (t0, t1, ...) for incremental tag construction
- Object fields for multi-position tags with optional trailing
- Enum union types for constrained positions
- Double cast (as unknown as) in dispatch for strict TS compat
- 13 unit tests covering interface/function gen and dispatch

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Integrate builder generation into the CLI pipeline:
- --builders <file> flag for standalone use
- Included in --all (default: builders.ts)
- Added to needKinds check and help text

Round-trip tests verify builder correctness:
- Synthetic tests for simple and multi-position tags
- Real-schema test cross-checks all 132 constrained kinds:
  generate sample data → simulate builder → validate with planner
- Skips literal-only tags the builder can't produce (e.g., "-")

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Three bugs in the builder pipeline:

[P1] Constant-only required tags (e.g., ["z", "order"] in kind 38383)
were silently dropped because buildBuilderTag() returned undefined when
no input positions existed. Now emits them unconditionally in the
function body with no interface field.

[P1] anyOf group semantics were erased — collectKindTags() flattens
groups into individual entries, so the planner emitted them as
individual optional_tag actions. Now consults shape.anyOfTagGroups
directly and emits any_of_group actions with runtime enforcement
("At least one of ... is required").

[P2] Round-trip test skipped require_tag checks when the builder didn't
produce that tag, masking both P1 bugs. Now checks ALL require_tag and
any_of_group actions. Also adds implied input positions when
minItems > positions.length (needed for kind 777 filter tags).

393 tests passing (was 385). 133 builders, tsc clean.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@alltheseas alltheseas changed the title feat: kind-scoped tag types (--kind-tags) feat: kind-scoped tag types + tag builder functions (issues #10, #11) Mar 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
tests/round-trip.test.ts (1)

331-349: This round-trip check never exercises the generated builders.

After creating buildersCode, the test switches back to planBuilder() plus simulateBuilder(). That means emitter/planner drift, invalid identifiers from schema-derived field names, and other codegen-only failures won't be caught here. I'd add a compile/smoke test against the emitted builders.ts artifact itself, not just the planner simulation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/round-trip.test.ts` around lines 331 - 349, The test currently
generates buildersCode via emitBuildersFile but never compiles or executes that
emitted artifact (it uses planBuilder/simulateBuilder instead), so add a
smoke/compile step that writes buildersCode to a temporary file and compiles or
imports it to exercise the real generated functions (e.g., ensure export
function buildKindTags and export function buildKind\d+Tags exist and can be
invoked). Specifically: after emitBuildersFile(buildersCode) and the existing
assertions, write buildersCode to a temp builders.ts, run a TypeScript compile
or dynamic import to ensure it parses, then for each shape that had
builderActions use the corresponding emitted builder (found by matching exported
buildKind\d+Tags or buildKindTags) with generateSampleData output to actually
call the generated function and verify it returns tags; keep using
planKindValidator to cross-check results. Ensure to cleanup temp file and
surface compile/runtime errors in test failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/plan-builders.ts`:
- Around line 199-205: The tuple-expansion logic currently derives implied
positions from req.positions.length (variables impliedCount, explicitInputCount,
impliedInputCount, totalInputCount), which breaks when positions carry sparse
PositionType.index values; instead compute the tuple length as (max
position.index)+1, sort req.positions by p.index, and iterate indices
0..maxIndex filling missing slots explicitly (treating missing entries as
implied positions) before generating names/inputs—update any loops that use
impliedCount or rely on array order (including the main loop handling 209-257)
to reference p.index and the filled/sorted position list.
- Around line 133-156: collectKindTags can drop "required" when a stricter
KindTagEntry overwrites the original source; update the Phase 1 loop so
requiredness is determined by the shape's required tag names (by tagName) not by
the surviving entry.source. Concretely, introduce or reuse a Set of required tag
names from shape.requiredTags and change the guard that currently reads like "if
(anyOfTagNames.has(entry.tagName) && entry.source !== 'required') continue" to
treat entries as required when entry.tagName is in the shape-required set (e.g.,
only defer if anyOfTagNames.has(entry.tagName) &&
!shapeRequiredTags.has(entry.tagName) ); likewise when adding actions, use
shapeRequiredTags.has(entry.tagName) (or the shape-derived set) to decide to
push a 'required_tag' and to add to requiredTagNames even if KindTagEntry.source
!== 'required'; keep using buildBuilderTag(entry) and
entryByTag/requiredTagNames sets as before.

---

Nitpick comments:
In `@tests/round-trip.test.ts`:
- Around line 331-349: The test currently generates buildersCode via
emitBuildersFile but never compiles or executes that emitted artifact (it uses
planBuilder/simulateBuilder instead), so add a smoke/compile step that writes
buildersCode to a temporary file and compiles or imports it to exercise the real
generated functions (e.g., ensure export function buildKindTags and export
function buildKind\d+Tags exist and can be invoked). Specifically: after
emitBuildersFile(buildersCode) and the existing assertions, write buildersCode
to a temp builders.ts, run a TypeScript compile or dynamic import to ensure it
parses, then for each shape that had builderActions use the corresponding
emitted builder (found by matching exported buildKind\d+Tags or buildKindTags)
with generateSampleData output to actually call the generated function and
verify it returns tags; keep using planKindValidator to cross-check results.
Ensure to cleanup temp file and surface compile/runtime errors in test failures.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 93bb64c1-f8b3-46de-877c-14e281867828

📥 Commits

Reviewing files that changed from the base of the PR and between 60da16e and b6dbed5.

📒 Files selected for processing (6)
  • src/emit-builders.ts
  • src/index.ts
  • src/plan-builders.ts
  • tests/emit-builders.test.ts
  • tests/plan-builders.test.ts
  • tests/round-trip.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/index.ts

Comment thread src/emit-builders.ts
Comment on lines +59 to +80
function emitInterfaceField(tag: BuilderTag, required: boolean): string {
const lines: string[] = [];
const inputPositions = tag.positions.filter(p => p.source === 'input');
const opt = required ? '' : '?';

if (isObjectField(tag)) {
// Object field for multi-position tags
lines.push(` /** ${tag.tagName} tag */`);
lines.push(` ${tag.fieldName}${opt}: {`);
for (const pos of inputPositions) {
const fname = pos.fieldName ?? 'value';
const ftype = emitFieldType(pos.inputType ?? { type: 'string' });
const posOpt = pos.required ? '' : '?';
lines.push(` ${fname}${posOpt}: ${ftype};`);
}
lines.push(' };');
} else {
// Simple field for single-input tags
const pos = inputPositions[0];
const ftype = emitFieldType(pos.inputType ?? { type: 'string' });
lines.push(` /** ${tag.tagName} tag */`);
lines.push(` ${tag.fieldName}${opt}: ${ftype};`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The generated input API can't represent tags whose presence is independent of a payload value.

For single-input tags, emitInterfaceField() treats the property as “the value”, while emitTagConstruction() also uses that same property as “should this tag exist”. That makes shapes like ["x"] | ["x", value] unrepresentable: optional tags can only be omitted or carry a value, and required tags incorrectly require one. Lines 216-218 then handle the zero-input case by promoting optional literal-only tags to unconditional output because there is no selector field at all. This needs a separate presence signal (or a union that models the bare form) instead of deriving tag presence solely from data.<field>.

Also applies to: 92-140, 213-219

Comment thread src/plan-builders.ts
Comment on lines +133 to +156
// Build a lookup from collectKindTags entries (has the more constrained version)
const entryByTag = new Map<string, KindTagEntry>();
for (const entry of entries) {
entryByTag.set(entry.tagName, entry);
}

// Track required tag names (auto-satisfy anyOf group constraints)
const requiredTagNames = new Set<string>();

// Phase 1: Emit non-anyOf tags
for (const entry of entries) {
// If this tag belongs to an anyOf group AND wasn't promoted to required,
// defer to group handling in phase 2
if (anyOfTagNames.has(entry.tagName) && entry.source !== 'required') continue;

const tag = buildBuilderTag(entry);
if (!tag) continue;

if (entry.source === 'required') {
actions.push({ type: 'required_tag', tag });
requiredTagNames.add(entry.tagName);
} else {
actions.push({ type: 'optional_tag', tag });
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't let collectKindTags() erase requiredness.

collectKindTags() keeps the most-constrained entry, not the most-required one. If a tag is present in shape.requiredTags and a stricter copy later comes from anyOfTagGroups or a conditional, the surviving KindTagEntry.source is no longer 'required'. Line 146 then skips it, and Lines 151-153 never add it to requiredTagNames, so a required tag gets downgraded to optional/group-only output. Requiredness should come from shape.requiredTags by tag name, not from the winning entry source.

Possible fix
 export function planBuilder(shape: KindShape): BuilderAction[] | undefined {
   const entries = collectKindTags(shape);
   if (entries.length === 0) return undefined;

   const actions: BuilderAction[] = [];
+  const requiredTagNames = new Set(shape.requiredTags.map(req => req.tagName));

   // Collect all tag names that belong to anyOf groups
   const anyOfTagNames = new Set<string>();
   for (const group of shape.anyOfTagGroups) {
     for (const req of group.requirements) {
       anyOfTagNames.add(req.tagName);
     }
   }

@@
-  // Track required tag names (auto-satisfy anyOf group constraints)
-  const requiredTagNames = new Set<string>();
-
   // Phase 1: Emit non-anyOf tags
   for (const entry of entries) {
+    const isRequired = requiredTagNames.has(entry.tagName);
     // If this tag belongs to an anyOf group AND wasn't promoted to required,
     // defer to group handling in phase 2
-    if (anyOfTagNames.has(entry.tagName) && entry.source !== 'required') continue;
+    if (anyOfTagNames.has(entry.tagName) && !isRequired) continue;

     const tag = buildBuilderTag(entry);
     if (!tag) continue;

-    if (entry.source === 'required') {
+    if (isRequired) {
       actions.push({ type: 'required_tag', tag });
-      requiredTagNames.add(entry.tagName);
     } else {
       actions.push({ type: 'optional_tag', tag });
     }
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plan-builders.ts` around lines 133 - 156, collectKindTags can drop
"required" when a stricter KindTagEntry overwrites the original source; update
the Phase 1 loop so requiredness is determined by the shape's required tag names
(by tagName) not by the surviving entry.source. Concretely, introduce or reuse a
Set of required tag names from shape.requiredTags and change the guard that
currently reads like "if (anyOfTagNames.has(entry.tagName) && entry.source !==
'required') continue" to treat entries as required when entry.tagName is in the
shape-required set (e.g., only defer if anyOfTagNames.has(entry.tagName) &&
!shapeRequiredTags.has(entry.tagName) ); likewise when adding actions, use
shapeRequiredTags.has(entry.tagName) (or the shape-derived set) to decide to
push a 'required_tag' and to add to requiredTagNames even if KindTagEntry.source
!== 'required'; keep using buildBuilderTag(entry) and
entryByTag/requiredTagNames sets as before.

Comment thread src/plan-builders.ts
Comment on lines +199 to +205
// Determine how many total positions we need (schema may imply more via minItems)
const impliedCount = Math.max(req.positions.length, req.minItems);

// Count non-literal positions after index 0 to determine naming strategy
const explicitInputCount = req.positions.filter((p, i) => i > 0 && p.constValue === undefined).length;
const impliedInputCount = impliedCount - req.positions.length;
const totalInputCount = explicitInputCount + impliedInputCount;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use PositionType.index, not array length/order, when expanding tuple positions.

This code mixes explicit tuple indices with raw array order. A requirement containing positions 0 and 2 will either miss slot 1 or duplicate slot 2, because impliedCount and the implied-position loop are based on req.positions.length instead of max(index) + 1, and the main loop emits whatever order the array happens to have. Sorting by pos.index and filling missing indices explicitly is needed here.

Possible fix
-  const impliedCount = Math.max(req.positions.length, req.minItems);
+  const explicitPositions = [...req.positions].sort((a, b) => a.index - b.index);
+  const impliedCount = Math.max(
+    explicitPositions.reduce((max, pos) => Math.max(max, pos.index + 1), 0),
+    req.minItems,
+  );

@@
-  for (const pos of req.positions) {
+  for (const pos of explicitPositions) {
     if (pos.index === 0) {
@@
-  for (let i = req.positions.length; i < impliedCount; i++) {
+  const occupied = new Set(explicitPositions.map(pos => pos.index));
+  for (let i = 1; i < impliedCount; i++) {
+    if (occupied.has(i)) continue;
     inputIndex++;
     const fieldName = useObjectField ? (inputIndex === 1 ? 'value' : `value${inputIndex}`) : undefined;
     positions.push({
       index: i,
       source: 'input',

Also applies to: 209-257

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plan-builders.ts` around lines 199 - 205, The tuple-expansion logic
currently derives implied positions from req.positions.length (variables
impliedCount, explicitInputCount, impliedInputCount, totalInputCount), which
breaks when positions carry sparse PositionType.index values; instead compute
the tuple length as (max position.index)+1, sort req.positions by p.index, and
iterate indices 0..maxIndex filling missing slots explicitly (treating missing
entries as implied positions) before generating names/inputs—update any loops
that use impliedCount or rely on array order (including the main loop handling
209-257) to reference p.index and the filled/sorted position list.

Floor the variant loop at minItems=1 so emitted tuples always include
at least the tag name position. Also lower the kind-tags type count
threshold from 300 to 290 to accommodate upstream schema changes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/emit-kind-tags.ts (1)

51-55: anyOf constraints not counted in score.

The constraintScore function doesn't count positions with anyOf constraints, even though emitPositionType (from context snippet 4) handles them as valid constraints. This could cause the deduplication logic to prefer a version without anyOf over one with it.

Consider adding p.anyOf && p.anyOf.length > 0 to the filter condition if anyOf should contribute to the score.

🔧 Proposed fix
 function constraintScore(req: TagRequirement): number {
   return req.positions.filter(p =>
-    p.constValue !== undefined || (p.enumValues && p.enumValues.length > 0) || p.pattern
+    p.constValue !== undefined || (p.enumValues && p.enumValues.length > 0) || p.pattern || (p.anyOf && p.anyOf.length > 0)
   ).length;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/emit-kind-tags.ts` around lines 51 - 55, The constraintScore function is
missing counting of positions that have anyOf constraints; update
constraintScore (used alongside emitPositionType) so the filter also treats
p.anyOf with length > 0 as a valid constraint (i.e., include p.anyOf &&
p.anyOf.length > 0 in the predicate) so versions with anyOf are scored correctly
during deduplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/emit-kind-tags.ts`:
- Around line 51-55: The constraintScore function is missing counting of
positions that have anyOf constraints; update constraintScore (used alongside
emitPositionType) so the filter also treats p.anyOf with length > 0 as a valid
constraint (i.e., include p.anyOf && p.anyOf.length > 0 in the predicate) so
versions with anyOf are scored correctly during deduplication.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e347da64-d7a7-42c4-a6e8-42582227229a

📥 Commits

Reviewing files that changed from the base of the PR and between b6dbed5 and d954dbb.

📒 Files selected for processing (2)
  • src/emit-kind-tags.ts
  • tests/compile-check.test.ts

alltheseas and others added 3 commits March 30, 2026 08:59
Generated output gets embedded in production binaries (C, Rust, Swift),
so the license must be permissive. Follows Rust ecosystem convention
(serde, tokio, etc.) of dual MIT/Apache-2.0.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The GPL LICENSE was added to main by mistake. The project is dual-licensed
under MIT and Apache-2.0 (see LICENSE-MIT and LICENSE-APACHE).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

Event builders: generate tag construction functions per kind Kind-scoped tag types: per-kind tag tuple types from kind schema constraints

1 participant