Skip to content

NON-BREAKING: fix docs discrepancies between README, AI skill, and lexicon schemas - #196

Merged
aspiers merged 16 commits into
mainfrom
fix/readme-lexicon-audit
Apr 8, 2026
Merged

NON-BREAKING: fix docs discrepancies between README, AI skill, and lexicon schemas#196
aspiers merged 16 commits into
mainfrom
fix/readme-lexicon-audit

Conversation

@aspiers

@aspiers aspiers commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Audited README.md and the building-with-hypercerts-lexicons AI skill against the actual lexicon JSON files and fixed all discrepancies found.

Fixes

High priority

  • validate() examples: Fixed parameter order (validate(record, id, hash) not validate(id, record)) and result shape (.success/.error not .valid/.errors) in Quick Start and Validation sections of both README and AI skill

Medium priority

  • Lexicon Map badge chain: Reversed arrow direction to match actual strongRef references (response -> award -> definition, not definition -> award -> response)
  • Lexicon Map workScope: Show cel -> tag as indirect (via usedTags), add string as the free-form alternative (was missing)
  • Lexicon Map context targets: Changed specific target lists to any record (...) since subjects are generic strongRef not limited to specific types
  • Lexicon Map CERTIFIED section: Added missing link/evm entry
  • Contributor Information: Fixed "avatar" to "image" (matching actual property name)
  • board.json schema: Fixed incorrect NSID org.hypercerts.claim.collection -> org.hypercerts.collection in description
  • AI skill relationship map: Added missing CERTIFIED section with badge chain, link/evm, location, actor types

Low priority

  • Badge Definition: Added "type" to summary (required badgeType field was omitted)
  • Location examples: Added $type discriminator to union members

Files changed

  • README.md — diagram, reference table, and code example fixes
  • .agents/skills/building-with-hypercerts-lexicons/SKILL.md — same fixes mirrored
  • lexicons/org/hyperboards/board.json — fix wrong NSID in description
  • SCHEMAS.md — auto-regenerated from board.json change

Summary by CodeRabbit

  • Documentation

    • Updated examples, diagrams, and wording to reflect lexicon changes (image vs avatar, new CERTIFIED relationships, generalized context links, revised work-scope options, and clarified example shapes).
  • Schema Updates

    • Clarified referenced collection types and added required union discriminators in example record shapes and docs.
  • Testing & Quality

    • Added automated checks to validate documentation code snippets, exported identifiers, and $type usages.
  • Tooling

    • Strengthened TypeScript checks and added a test-focused TS config.

Audit README.md and the building-with-hypercerts-lexicons AI skill
against the actual lexicon JSON files and fix all discrepancies found:

- Fix validate() examples: correct parameter order and result shape
- Fix Lexicon Map diagram: reverse badge chain arrows to match actual
  strongRef direction, show workScope string alternative alongside CEL,
  show CEL->tag relationship as indirect via usedTags, add link/evm
  to CERTIFIED section, use generic 'any record' for context targets
- Fix 'avatar' -> 'image' in contributorInformation descriptions
- Add 'badgeType' to badge definition summary (required field)
- Add $type discriminator to location example union members
- Fix board.json schema description: wrong collection NSID
  (org.hypercerts.claim.collection -> org.hypercerts.collection)
- Add CERTIFIED section to AI skill relationship map (was missing)
@changeset-bot

changeset-bot Bot commented Apr 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cacc857

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hypercerts-org/lexicon Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Apr 4, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Docs and examples updated to match lexicon/schema changes (validate() signature, NSID names, union discriminators, contributor field); a Vitest test suite was added to validate TypeScript code blocks in docs against generated exports; TypeScript configs and npm scripts adjusted; minor schema description edits applied.

Changes

Cohort / File(s) Summary
Documentation & Examples
.agents/skills/building-with-hypercerts-lexicons/SKILL.md, README.md
Updated example validate() call signature/result shape; replaced NSID constant names (e.g., HYPERCERTS_COLLECTION_NSID, CONTEXT_ATTACHMENT_NSID, FUNDING_RECEIPT_NSID); renamed contributor avatarimage; generalized context relationship semantics; added/updated $type discriminators and union variants; added CERTIFIED section.
Schema Docs & Lexicon
SCHEMAS.md, lexicons/org/hyperboards/board.json
Adjusted org.hyperboards.board.subject description to reference org.hypercerts.collection; minor formatting/reflow in schema docs.
Test Infrastructure
tests/validate-doc-snippets.test.ts
Added Vitest test that extracts TypeScript fenced code blocks from docs and validates imports, exported symbols, NSID usage, $type strings, namespace members, and forbids deprecated validate(...) call patterns against generated/exports.js and generated/lexicons.js.
Docs Process
AGENTS.md
Require synchronizing lexicon documentation across both README.md and the skill doc; document that npm run test runs the doc-snippet validator.
TypeScript Config & Scripts
package.json, tsconfig.json, tsconfig.test.json
typecheck script now runs tsc --noEmit and tsc --noEmit -p tsconfig.test.json; added @types/node devDependency; removed tests/**/* from main tsconfig.json include; added tsconfig.test.json targeting generated/**, tests/**, and lexicons/**/*.json.
Changeset & CI guidance
.changeset/*, .claude/skills/writing-changesets/SKILL.md
Added changeset documenting doc/schema corrections and refined guidance on when changesets are required for package-shipped files and README.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Doc as Documentation (README / SKILL.md)
  participant Test as Vitest Runner
  participant Exports as generated/exports.js
  participant Lexicons as generated/lexicons.js

  Doc->>Test: Extract TypeScript fenced code blocks
  Test->>Exports: Import runtime package exports
  Test->>Lexicons: Import lexicon schema defs
  Test->>Exports: Verify imported symbols, NSID constants, namespace members
  Test->>Lexicons: Verify `$type`/schema ids referenced in docs
  Note over Test,Doc: Fail on deprecated validate(...) patterns or missing exports
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • s-adamantine
  • holkexyz
  • Kzoeps

Poem

🐇 I hopped through docs and nudged each line,

NSIDs and $types now neatly align.
Tests nibble snippets with curious cheer,
Exports and lexicons all drawn near.
A tiny hop for code, a joyful ear.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Lexicons Styleguide Compliance ⚠️ Warning Three lexicon files violate styleguide compliance: strongRef descriptions lack required keywords or inline documentation. Update descriptions in board.json, acknowledgement.json, and evaluation.json to include 'lexicon'/'conform' keywords or add inline explanations for generic strongRef design choices.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Lexicon Documentation Sync ✅ Passed Lexicon documentation synchronization is properly maintained with subject field description correctly updated and reflected across all documentation files.
Title check ✅ Passed The title accurately summarizes the main objective of the PR—fixing documentation discrepancies across README, AI skill, and lexicon schemas—and is specific enough to be meaningful without being overly detailed.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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 fix/readme-lexicon-audit

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.

Add tests/validate-skill-snippets.test.ts which auto-extracts all
TypeScript code blocks from the AI skill SKILL.md and validates:
- All named/namespace imports resolve to real package exports
- All *_NSID references map to exported constants
- Record construction snippets pass schema validation
- validate() and validateMain() work as documented

Bugs found and fixed in SKILL.md:
- COLLECTION_NSID -> HYPERCERTS_COLLECTION_NSID
- ATTACHMENT_NSID -> CONTEXT_ATTACHMENT_NSID
- RECEIPT_NSID -> FUNDING_RECEIPT_NSID
- ACKNOWLEDGEMENT_NSID -> CONTEXT_ACKNOWLEDGEMENT_NSID
- Acknowledgement context union needs $type discriminator
- Attachment content union items need $type discriminator

Also adds tsconfig.test.json extending the main tsconfig with
@types/node for test files, keeping the library tsconfig
runtime-agnostic.
@aspiers
aspiers marked this pull request as ready for review April 4, 2026 11:18
Copilot AI review requested due to automatic review settings April 4, 2026 11:18
@aspiers
aspiers marked this pull request as draft April 4, 2026 11:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns repository documentation and the “building-with-hypercerts-lexicons” AI skill with the actual lexicon schemas, and adds automated checks to prevent the skill snippets from drifting from the package API over time.

Changes:

  • Corrected README + SKILL.md examples/diagrams (notably validate() call shape and several relationship-map entries).
  • Fixed a wrong NSID reference in org.hyperboards.board and regenerated SCHEMAS.md.
  • Added a Vitest suite that validates SKILL.md snippets against generated exports, plus TS config changes to typecheck tests separately (with Node typings).

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tsconfig.test.json New TS config to typecheck tests with Node types and no emit.
tsconfig.json Removes tests/**/* from the default project include set.
package.json Extends typecheck to also run tsc -p tsconfig.test.json; adds @types/node.
package-lock.json Locks @types/node and transitive undici-types.
tests/validate-skill-snippets.test.ts New test validating SKILL.md imports/snippets against generated exports + runtime validation.
README.md Fixes diagrams and validate() examples to match actual API/schemas.
.agents/skills/building-with-hypercerts-lexicons/SKILL.md Mirrors README fixes and updates snippet imports/record examples.
lexicons/org/hyperboards/board.json Fixes incorrect NSID in the subject field description.
SCHEMAS.md Regenerated docs reflecting the board.json description fix.
Comments suppressed due to low confidence (1)

.agents/skills/building-with-hypercerts-lexicons/SKILL.md:459

  • The funding receipt example uses fields (subject, paidAt) that are not part of the org.hypercerts.funding.receipt schema (see lexicons/org/hypercerts/funding/receipt.json). This snippet should be updated to only use schema fields (e.g. for and/or occurredAt if you want to link to an activity and record payment time) so downstream users aren’t copy/pasting an invalid record shape.
const receipt = {
  $type: FUNDING_RECEIPT_NSID,
  subject: {
    uri: "at://did:plc:alice/org.hypercerts.claim.activity/abc123",
    cid: "...",
  },
  to: "did:plc:recipient",
  amount: "1000.00",
  currency: "USD",
  paymentRail: "ethereum",
  transactionId: "0xabc...",
  paidAt: new Date().toISOString(),
  createdAt: new Date().toISOString(),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/validate-skill-snippets.test.ts Outdated
Rename validate-skill-snippets.test.ts -> validate-doc-snippets.test.ts
and extend it to auto-check both SKILL.md and README.md TypeScript
code blocks.

New auto-extracted checks:
- UPPER_SNAKE_CASE identifiers in code bodies (not just imports)
- $type string literals match known lexicon NSIDs/defs
- Dotted accesses on mapping objects (HYPERCERTS_NSIDS.X etc.)
- Runtime namespace property accesses (Namespace.validateMain etc.)
- validate() call signature correctness (catches swapped args)

README.md bugs found and fixed (same categories as SKILL.md):
- COLLECTION_NSID -> HYPERCERTS_COLLECTION_NSID
- ACKNOWLEDGEMENT_NSID -> CONTEXT_ACKNOWLEDGEMENT_NSID
- ATTACHMENT_NSID -> CONTEXT_ATTACHMENT_NSID
- Acknowledgement context union missing $type discriminator
- Attachment content union items missing $type discriminator
Update AGENTS.md 'Adding/modifying a lexicon' checklist to require
both README.md and SKILL.md to be updated when either already
references the changed lexicon. Also mention the auto-checking test.

Fix funding receipt snippet in SKILL.md: replace non-existent fields
'subject' (strongRef) and 'paidAt' with actual schema fields 'for'
(at-uri) and 'occurredAt' (datetime).
@aspiers
aspiers marked this pull request as ready for review April 4, 2026 14:16
Copilot AI review requested due to automatic review settings April 4, 2026 14:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/validate-doc-snippets.test.ts
Comment thread tests/validate-doc-snippets.test.ts
Fix stateful /g regex bug flagged by Copilot: exec() on a global
regex carries lastIndex across loop iterations, which could skip
matches at the start of later code blocks. Reset lastIndex = 0
before each block iteration in collectMappingAccesses() and
collectRuntimeNamespaceAccesses().

@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: 2

🧹 Nitpick comments (1)
tests/validate-doc-snippets.test.ts (1)

252-356: This guardrail still won't catch snippet field drift.

The extracted checks validate exports, $type literals, and validate() argument order, but they never typecheck or schema-validate the example object literals themselves. A snippet can still use nonexistent fields and pass, which leaves the same class of docs regression as the funding-receipt subject/paidAt mismatch uncovered.

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

In `@tests/validate-doc-snippets.test.ts` around lines 252 - 356, The tests never
actually schema-validate example object literals extracted from snippets, so add
a new check that finds literal objects in TypeScript blocks and runs the
appropriate runtime validate() for them: implement a helper (e.g.
collectExampleObjectLiterals or extend collectTypeScriptBlocks) that returns {
obj, nsidOrType, context } for each literal with a $type or NSID-like marker,
then in describeDocSnippets add a new "example object literals validate" suite
which, for each entry, looks up the validate function (from pkgExports or
exportsForSource) and calls validate(obj, nsidOrType) (or validate(obj) if your
validate API expects only the record) and expects no thrown errors; include the
context in any error message to aid debugging. Ensure you reuse existing symbols
like validate(), pkgExports, exportsForSource, and the block-collection helpers
(collectTypeScriptBlocks / collect* functions) so the new test integrates with
the current extraction pipeline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@AGENTS.md`:
- Around line 242-249: The AGENTS.md update relaxes documentation requirements
by allowing any undocumented lexicon to skip README.md/SKILL.md updates; change
the wording so that README.md and .agents/skills/.../SKILL.md updates remain
mandatory for any newly added or modified required lexicons (only allow the
README/SKILL exemption for lexicons explicitly marked optional). Locate the
section that currently starts "Update `README.md` and `SKILL.md` as appropriate"
and revise it to: require README.md and SKILL.md updates when adding or changing
a required lexicon (document all properties except facet fields), and only
permit skipping docs for lexicons flagged as optional; ensure references to
"lexicon" and the path
`.agents/skills/building-with-hypercerts-lexicons/SKILL.md` are updated to
reflect this stricter rule.

In `@tests/validate-doc-snippets.test.ts`:
- Around line 47-55: The extractTypeScriptBlocks function currently only matches
exact lowercase "```typescript" with LF endings, so fences like "```ts",
"```tsx", info-string suffixes, or CRLF are skipped; update the regex in
extractTypeScriptBlocks to use a case-insensitive pattern that accepts
ts|tsx|typescript, allows word-boundary and optional info-string chars, and
permits \r?\n before the captured body (e.g.
/```(?:ts|tsx|typescript)\b[^\n]*\r?\n([\s\S]*?)```/gi) so all TypeScript fences
are detected and their contents pushed to blocks.

---

Nitpick comments:
In `@tests/validate-doc-snippets.test.ts`:
- Around line 252-356: The tests never actually schema-validate example object
literals extracted from snippets, so add a new check that finds literal objects
in TypeScript blocks and runs the appropriate runtime validate() for them:
implement a helper (e.g. collectExampleObjectLiterals or extend
collectTypeScriptBlocks) that returns { obj, nsidOrType, context } for each
literal with a $type or NSID-like marker, then in describeDocSnippets add a new
"example object literals validate" suite which, for each entry, looks up the
validate function (from pkgExports or exportsForSource) and calls validate(obj,
nsidOrType) (or validate(obj) if your validate API expects only the record) and
expects no thrown errors; include the context in any error message to aid
debugging. Ensure you reuse existing symbols like validate(), pkgExports,
exportsForSource, and the block-collection helpers (collectTypeScriptBlocks /
collect* functions) so the new test integrates with the current extraction
pipeline.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9298c275-5f17-4dcc-9016-11c5ea633110

📥 Commits

Reviewing files that changed from the base of the PR and between a368a9f and 44c0343.

📒 Files selected for processing (4)
  • .agents/skills/building-with-hypercerts-lexicons/SKILL.md
  • AGENTS.md
  • README.md
  • tests/validate-doc-snippets.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • README.md

Comment thread AGENTS.md
Comment thread tests/validate-doc-snippets.test.ts

@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 (3)
tests/validate-doc-snippets.test.ts (3)

166-220: Consider adding a clarifying comment for the dual-pass logic.

The function uses two passes with different regexes to distinguish runtime vs type-annotation positions. The edge case handling at lines 208-212 (marking as seen when prefix indicates type position but key isn't in typePositions) could benefit from a brief inline comment explaining why this path exists.

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

In `@tests/validate-doc-snippets.test.ts` around lines 166 - 220, The dual-pass
logic in collectRuntimeNamespaceAccesses uses pattern and typeContextPattern to
first record all type-only occurrences into typePositions and then skip
type-only matches during the runtime pass; add a brief inline comment above the
block that checks prefix/isTypePosition (around the branch that uses seen and
typePositions) explaining that we mark the key in seen when the current match
appears to be in a type context so we don't report it later or re-check it, and
that this branch only skips reporting here if the same key appears in
typePositions (i.e., appears elsewhere in a type position) — reference
collectRuntimeNamespaceAccesses, pattern, typeContextPattern, typePositions,
seen, prefix, and isTypePosition.

344-357: Redundant double assertion logic.

The test uses expect.soft(...).not.toMatch(...) at line 348, then immediately re-checks the same condition and throws a manual error at lines 349-355. The soft assertion already captures the failure; the manual throw is redundant and makes the test harder to follow.

♻️ Simplify to a single assertion
       it("no validate() calls have an NSID as the first argument", () => {
         for (const { firstArg, context } of suspicious) {
-          // This fails if someone writes validate(ACTIVITY_NSID, record, ...)
-          // instead of validate(record, ACTIVITY_NSID, ...)
-          expect.soft(firstArg).not.toMatch(/^[A-Z_]+_NSID$/);
-          if (firstArg.match(/^[A-Z_]+_NSID$/)) {
-            // Provide helpful context in the error
-            throw new Error(
+          expect(
+            firstArg,
               `validate() called with NSID "${firstArg}" as first argument ` +
-                `(should be second). Context: ...${context}...`,
-            );
-          }
+                `(should be second). Context: ...${context}...`
+          ).not.toMatch(/^[A-Z_]+_NSID$/);
         }
       });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/validate-doc-snippets.test.ts` around lines 344 - 357, The test
duplicates the same check: keep the soft assertion and remove the redundant
if-block that re-checks firstArg and throws; specifically delete the manual
conditional that inspects firstArg and throws an Error (the block referencing
firstArg and context) so the loop over suspicious (and the
expect.soft(firstArg).not.toMatch(/^[A-Z_]+_NSID$/)) is the single assertion
enforcing that validate() calls don't have an NSID as the first argument.

32-42: Redundant add call for "main" def.

Line 34 adds s.id unconditionally, and line 37 adds it again when defName === "main". The second add is a no-op since it's a Set, but this adds unnecessary confusion.

♻️ Suggested simplification
 for (const [, schema] of Object.entries(LexiconsExports.schemaDict)) {
   const s = schema as { id: string; defs: Record<string, unknown> };
   ALL_KNOWN_TYPE_STRINGS.add(s.id);
   for (const defName of Object.keys(s.defs)) {
-    if (defName === "main") {
-      ALL_KNOWN_TYPE_STRINGS.add(s.id); // main $type is just the NSID
-    } else {
+    if (defName !== "main") {
       ALL_KNOWN_TYPE_STRINGS.add(`${s.id}#${defName}`);
     }
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/validate-doc-snippets.test.ts` around lines 32 - 42, The loop over
Object.entries(LexiconsExports.schemaDict) redundantly adds s.id twice for
schemas with a "main" def; remove the conditional addition inside the if branch
and only add s.id once per schema and add `${s.id}#${defName}` for non-main
defs. Update the block that manipulates ALL_KNOWN_TYPE_STRINGS (referencing
schemaDict, s.id, defName, and ALL_KNOWN_TYPE_STRINGS) so s.id is added once
before iterating defs and the inner loop only adds `${s.id}#${defName}` for
defName !== "main".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/validate-doc-snippets.test.ts`:
- Around line 166-220: The dual-pass logic in collectRuntimeNamespaceAccesses
uses pattern and typeContextPattern to first record all type-only occurrences
into typePositions and then skip type-only matches during the runtime pass; add
a brief inline comment above the block that checks prefix/isTypePosition (around
the branch that uses seen and typePositions) explaining that we mark the key in
seen when the current match appears to be in a type context so we don't report
it later or re-check it, and that this branch only skips reporting here if the
same key appears in typePositions (i.e., appears elsewhere in a type position) —
reference collectRuntimeNamespaceAccesses, pattern, typeContextPattern,
typePositions, seen, prefix, and isTypePosition.
- Around line 344-357: The test duplicates the same check: keep the soft
assertion and remove the redundant if-block that re-checks firstArg and throws;
specifically delete the manual conditional that inspects firstArg and throws an
Error (the block referencing firstArg and context) so the loop over suspicious
(and the expect.soft(firstArg).not.toMatch(/^[A-Z_]+_NSID$/)) is the single
assertion enforcing that validate() calls don't have an NSID as the first
argument.
- Around line 32-42: The loop over Object.entries(LexiconsExports.schemaDict)
redundantly adds s.id twice for schemas with a "main" def; remove the
conditional addition inside the if branch and only add s.id once per schema and
add `${s.id}#${defName}` for non-main defs. Update the block that manipulates
ALL_KNOWN_TYPE_STRINGS (referencing schemaDict, s.id, defName, and
ALL_KNOWN_TYPE_STRINGS) so s.id is added once before iterating defs and the
inner loop only adds `${s.id}#${defName}` for defName !== "main".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c5af6709-79df-4f1f-8f13-8e3519d88576

📥 Commits

Reviewing files that changed from the base of the PR and between 44c0343 and a467d52.

📒 Files selected for processing (1)
  • tests/validate-doc-snippets.test.ts

@aspiers
aspiers requested a review from s-adamantine April 6, 2026 13:17
aspiers added 5 commits April 6, 2026 18:44
Accept ```ts, ```tsx, and ```typescript (case-insensitive), plus
optional info-string suffixes and CRLF line endings. The previous
pattern only matched exact lowercase ```typescript with LF, so any
other common fence variant was silently skipped and never validated.
Tighten the 'recommended but not mandatory' exemption so it only
applies to optional or internal-only lexicons. Any newly added or
modified required/public lexicon must be documented in both README.md
and SKILL.md, as flagged by CodeRabbit.
Replace the expect.soft() + if/throw pair with a single expect(value,
message).not.toMatch() call. The soft assertion and the manual throw
were checking the same condition twice; a single assertion with the
context message in the second argument is cleaner and sufficient.
s.id is already added unconditionally before the inner loop; the
if/else branch that re-added it for 'main' defs was a no-op (Set
deduplicates). Simplify to a single if-not-main guard that only adds
the nsid#defName fragment form for non-main defs.
The type-position branch has a subtle two-sub-case structure that
confused CodeRabbit. Expand the comment to explain: when typePositions
contains the key, we skip without marking seen so a later runtime
occurrence can still be reported; when it doesn't, we mark seen to
prevent repeated edge-case re-examination then skip.
@aspiers aspiers changed the title docs: fix discrepancies between README, AI skill, and lexicon schemas NON-BREAKING: fix docs discrepancies between README, AI skill, and lexicon schemas Apr 7, 2026
Copilot AI review requested due to automatic review settings April 7, 2026 23:04
@aspiers

aspiers commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews. Here's the current status of the outstanding review comments:

Addressed in code (in this PR):

  • Copilot: funding receipt subject/paidAt fields — fixed in 44c0343
  • Copilot: collectMappingAccesses() regex lastIndex — fixed in a467d52
  • Copilot: collectRuntimeNamespaceAccesses() regex lastIndex — fixed in a467d52
  • CodeRabbit: AGENTS.md doc-sync weakening — tightened in cb571fe
  • CodeRabbit: extractTypeScriptBlocks() only matching ts|tsx|typescript with LF — broadened in 1ff4a2d
  • CodeRabbit nitpick: dual-pass comment in collectRuntimeNamespaceAccesses() — added in 30ccade
  • CodeRabbit nitpick: redundant Set.add('main') — simplified in fafe230
  • CodeRabbit nitpick: redundant double-assertion in validate() first-arg test — simplified in 1452f9e

Deferred as follow-up:

  • CodeRabbit nitpick: schema-validate object literals in doc snippets to catch field-name drift like the funding receipt bug — filed as test: schema-validate object literals in doc snippets #202. The check is worth having but requires a non-trivial extraction pipeline (either regex + bracket counting or a lightweight TS parser), so I'd rather scope it separately than expand this PR further.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/validate-doc-snippets.test.ts Outdated

@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: 1

🧹 Nitpick comments (2)
tests/validate-doc-snippets.test.ts (2)

411-419: Also assert the failure-side error field.

This only proves validate() returns success: false. A regression in the documented .success/.error result shape would still slip through here.

💡 Proposed fix
   it("validate() fails for an invalid record", () => {
     const result = PackageExports.validate(
       { title: 123 },
       PackageExports.ACTIVITY_NSID,
       "main",
       false,
     );
     expect(result.success).toBe(false);
+    if (!result.success) {
+      expect(result.error).toBeDefined();
+    }
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/validate-doc-snippets.test.ts` around lines 411 - 419, The test
currently only checks result.success is false; update the test that calls
PackageExports.validate (using PackageExports.ACTIVITY_NSID and "main") to also
assert the failure-side result.error field is present and has the expected shape
(e.g., truthy / not null and/or contains an error message or array). Locate the
test block that assigns the validation output to the variable result and add an
assertion such as expect(result.error).toBeTruthy() or a more specific shape
check depending on the expected error type to guard against regressions in the
.success/.error result shape.

377-381: Use a non-validate-* filename for this docs-wide suite.

This test covers multiple documentation files rather than a single lexicon, so keeping it under the per-lexicon validate-* pattern makes that convention ambiguous. Renaming it to something like tests/doc-snippets.test.ts would keep the intent clearer. As per coding guidelines, "Create test files in tests/ directory with one file per lexicon, named validate-<lexicon-slug>.test.ts".

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

In `@tests/validate-doc-snippets.test.ts` around lines 377 - 381, The test file
name follows the per-lexicon `validate-*` convention but actually covers
multiple docs; rename the file from validate-doc-snippets.test.ts to a clearer
non-`validate-` name (e.g., doc-snippets.test.ts) and update any references or
test-runner configs accordingly so the suite using
describeDocSnippets("SKILL.md", ... ) and describeDocSnippets("README.md", ...)
remains discoverable; ensure no import paths or CI/test scripts still reference
the old filename.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/validate-doc-snippets.test.ts`:
- Around line 78-86: collectNamedImports is treating inline TypeScript "type"
modifiers as real import names; update the parsing in collectNamedImports to
strip a leading "type" token before deriving the import name and key.
Specifically, where the code currently computes name via
part.trim().split(/\s+as\s+/)[0].trim(), first normalize part by removing any
leading "type " (e.g., part = part.trim().replace(/^type\s+/, "")), then
continue with the existing split and trim, and use that sanitized name when
constructing key and pushing into imports (and ensure you still skip empty or
comment names as before).

---

Nitpick comments:
In `@tests/validate-doc-snippets.test.ts`:
- Around line 411-419: The test currently only checks result.success is false;
update the test that calls PackageExports.validate (using
PackageExports.ACTIVITY_NSID and "main") to also assert the failure-side
result.error field is present and has the expected shape (e.g., truthy / not
null and/or contains an error message or array). Locate the test block that
assigns the validation output to the variable result and add an assertion such
as expect(result.error).toBeTruthy() or a more specific shape check depending on
the expected error type to guard against regressions in the .success/.error
result shape.
- Around line 377-381: The test file name follows the per-lexicon `validate-*`
convention but actually covers multiple docs; rename the file from
validate-doc-snippets.test.ts to a clearer non-`validate-` name (e.g.,
doc-snippets.test.ts) and update any references or test-runner configs
accordingly so the suite using describeDocSnippets("SKILL.md", ... ) and
describeDocSnippets("README.md", ...) remains discoverable; ensure no import
paths or CI/test scripts still reference the old filename.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5eb03448-1ffa-44c1-bcfb-c7fead4c3869

📥 Commits

Reviewing files that changed from the base of the PR and between a467d52 and 30ccade.

📒 Files selected for processing (2)
  • AGENTS.md
  • tests/validate-doc-snippets.test.ts
✅ Files skipped from review due to trivial changes (1)
  • AGENTS.md

Comment thread tests/validate-doc-snippets.test.ts
aspiers added 2 commits April 7, 2026 23:31
Fix incorrect NSID reference in org.hyperboards.board subject field
description (org.hypercerts.claim.collection → org.hypercerts.collection).

Documentation fixes in README.md and SCHEMAS.md:
- Correct validate() call signature examples (parameter order and result shape)
- Fix relationship diagram arrow directions and missing entries
- Fix contributor field name (avatar → image)
- Fix context target descriptions (generalized to 'any record')
- Add missing $type discriminators to union member examples
The 'skip for documentation only' guidance was ambiguous. Explicitly
list the files included in the npm package (dist/, lexicons/,
SCHEMAS.md, CHANGELOG.md, and README.md which npm includes
automatically) so agents don't mistakenly skip changesets for changes
to those files.
@aspiers
aspiers force-pushed the fix/readme-lexicon-audit branch from e344803 to c2a52f7 Compare April 7, 2026 23:32
Copilot AI changed the title NON-BREAKING: fix docs discrepancies between README, AI skill, and lexicon schemas docs: explain why .js extension imports resolve correctly in TypeScript ESM tests Apr 7, 2026
import { type Foo } is valid TypeScript but 'type' is not a runtime
export. Without this guard, such imports would be looked up against the
package exports and fail with a misleading 'type is not exported' error.
Flagged by CodeRabbit.
Copilot AI review requested due to automatic review settings April 7, 2026 23:39
The building-with-hypercerts-lexicons skill is installed by downstream
AI agents and is their primary usage guide, making it effectively
user-facing. Changesets should cover changes to it, not just changes
to files in the npm package.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/validate-doc-snippets.test.ts
Comment thread package.json Outdated
aspiers added 2 commits April 7, 2026 23:54
moduleResolution: bundler resolves extensionless imports correctly, so
there is no need for explicit extensions. Extensionless is simpler and
the normal convention for non-ESM-output contexts like tests.
CI runs Node 20 but @types/node was ^25.5.2, meaning code could
typecheck against APIs that don't exist in the runtime being tested.
Flagged by Copilot.
Copilot AI review requested due to automatic review settings April 8, 2026 00:01
@aspiers
aspiers enabled auto-merge April 8, 2026 00:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread package.json
@aspiers aspiers changed the title docs: explain why .js extension imports resolve correctly in TypeScript ESM tests docs: fix discrepancies between README, AI skill, and lexicon schemas Apr 8, 2026
@aspiers aspiers changed the title docs: fix discrepancies between README, AI skill, and lexicon schemas NON-BREAKING: fix docs discrepancies between README, AI skill, and lexicon schemas Apr 8, 2026
@aspiers

aspiers commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Copilot incorrectly completely rewrote the PR title and body. I got Opus to restore the previous version.

@aspiers
aspiers merged commit 2aecfa6 into main Apr 8, 2026
10 checks passed
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.

4 participants