feat: add adjacent relationships and selfRefField to hierarchy edges#52
Conversation
Code reviewFound 4 issues:
https://github.com/rw Ньюburn/ost-tools/blob/78ebaa8ed24b4c105afaf3390226c41d70c4d6bf/src/wikilink-utils.ts#L24-L38
https://github.com/rw Ньюburn/ost-tools/blob/78ebaa8ed24b4c105afaf3390226c41d70c4d6bf/src/parse-embedded.ts#L181-L188
https://github.com/rw Ньюburn/ost-tools/blob/78ebaa8ed24b4c105afaf3390226c41d70c4d6bf/src/types.ts#L75-L76
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Introduce `$metadata.relationships` for defining adjacent (non-hierarchy) links between node types, with support for `format`, `matchers`, `fieldOn`, `field`, `multi`, and `embeddedTemplateFields` - Add `selfRefField` to hierarchy level config to allow a separate field for same-type parent relationships (distinct from the regular `field`) - Extend embedded parsing to recognise relationship headings and type list items and table rows beneath them as the relationship's child type, without requiring explicit inline annotations - Add `wikilink-utils.ts` and replace `resolve-links.ts` with `resolve-hierarchy-edges.ts` to cleanly separate wikilink parsing from edge resolution - Update `template-sync` to generate embedded sections (headings, lists, tables) from relationship definitions - Validate relationship wikilinks during the validate command - Update schemas, metaschema, README, docs, and skill references throughout
78ebaa8 to
315e936
Compare
|
Claude encountered an error —— View job Code ReviewI've thoroughly reviewed PR #52. Here's my analysis of the issues mentioned in the previous review, along with additional findings: Analysis of Previously Reported Issues✅ Issue 1: buildTargetIndex normalization - FIXEDThe current implementation in
The related tests ( ✅ Issue 2: Array mutation silent overwrite - FIXEDThe current implementation in } else {
// Field exists but is not an array - this is an error
throw new Error(
`Cannot append child link to field '${fieldName}' on node '${parentFieldAppend.node.label}': ` +
`field exists but is not an array (found ${typeof fieldValue}). ` +
`Child link: ${linkRef}`,
);
}This aligns with CLAUDE.md guidance: "fail loudly so we can fix problems." ✅ Issue 4: Missing documentation - NOT AN ISSUEThe
This documentation is present and well-maintained. 🔴 Critical Issue: Field Name Mismatch (aliases vs typeAliases)Location: There is a breaking inconsistency between the JSON schema and TypeScript types:
Impact: The code at if (metadata.typeAliases && Object.keys(metadata.typeAliases).length > 0) {
const aliasParts = Object.entries(metadata.typeAliases).map(([k, v]) => `${k} → ${v}`);But the actual field name in the JSON schema is Evidence from schema files:
Fix: The TypeScript interface in Additional ObservationsStrengths
Minor Suggestions
SummaryStatus: The PR implements valuable features for adjacent relationships and same-type parent references, but contains a breaking field name mismatch ( Recommended Action: Fix the field name inconsistency before merging. Note: This review is based on static code analysis. The CI environment's pre-commit hooks couldn't run due to missing dependencies ( |
Summary
$metadata.relationshipsfor defining adjacent (non-hierarchy) links between node types, with support forformat,matchers,fieldOn,field,multi, andembeddedTemplateFieldspropertiesselfRefFieldto hierarchy level config, enabling a separate field for same-type parent relationships distinct from the regular parentfieldresolve-links.tswithresolve-hierarchy-edges.tsand extractswikilink-utils.tsfor cleaner separation of concernstemplate-syncto generate embedded sections (headings, lists, tables) driven by relationship definitionsvalidatecommandTest plan
bun run testpasses (unit tests cover relationship parsing, hierarchy edge resolution, template-sync, and validation)bun run test:smokepasses against all registered spaces inconfig.jsonrelationshipsarray validates correctly viaost-tools schemas validateost-tools dumptemplate-syncgenerates correct embedded sections for schemas with relationships