feat: kind-scoped tag types + tag builder functions (issues #10, #11)#14
feat: kind-scoped tag types + tag builder functions (issues #10, #11)#14alltheseas wants to merge 12 commits into
Conversation
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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughAdds 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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/emit-kind-tags.ts (1)
135-144: Optional: guardminItems = 0in the optional trailing case to avoid emitting empty tuple variants.The loop at line 137 will generate variants starting from
len = minItems. WhileminItems = 0is theoretically possible in a schema, extraction logic consistently defaultsminItemsto either the number of defined positions or 2, so this is unlikely in practice. However, if it occurs, the resultingreadonly []type would be semantically incorrect for Nostr tags. Consider usingMath.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
📒 Files selected for processing (5)
src/emit-kind-tags.tssrc/emit-typescript.tssrc/index.tstests/compile-check.test.tstests/emit-kind-tags.test.ts
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>
There was a problem hiding this comment.
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 toplanBuilder()plussimulateBuilder(). 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 emittedbuilders.tsartifact 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
📒 Files selected for processing (6)
src/emit-builders.tssrc/index.tssrc/plan-builders.tstests/emit-builders.test.tstests/plan-builders.test.tstests/round-trip.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/index.ts
| 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};`); |
There was a problem hiding this comment.
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
| // 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 }); | ||
| } |
There was a problem hiding this comment.
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.
| // 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; |
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/emit-kind-tags.ts (1)
51-55:anyOfconstraints not counted in score.The
constraintScorefunction doesn't count positions withanyOfconstraints, even thoughemitPositionType(from context snippet 4) handles them as valid constraints. This could cause the deduplication logic to prefer a version withoutanyOfover one with it.Consider adding
p.anyOf && p.anyOf.length > 0to the filter condition ifanyOfshould 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
📒 Files selected for processing (2)
src/emit-kind-tags.tstests/compile-check.test.ts
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>
Summary
Kind-scoped tag types (
--kind-tags, issue #10)emit-kind-tags.tsemitter generates per-kind tag tuple types fromKindShape[]— 312 types across 177 kinds (e.g.,Kind30166_NTag,Kind10002_RTag)--kind-tags <file>CLI flag (included in--allaskind-tags.d.ts)p→PTag,P→Upper_PTag)Tag builder functions (
--builders, issue #11)plan-builders.ts) produces language-independentBuilderAction[]IR fromKindShapeemit-builders.ts) renders TypeScript interfaces + functions + dispatch--builders <file>flag (included in--all)["z", "order"]) emitted unconditionally without interface fieldsminItems > positions.lengthGenerated API example
Closes #10, closes #11
Test plan
builders.tsandkind-tags.d.tscompile withtsc --strict --noEmit🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--allemits both and logs generated counts.Tests
Chores