Skip to content

Unify hierarchy and relationship edges into a single labelled graph#57

Merged
mindsocket merged 1 commit into
mainfrom
feat/56-relationships-and-hierarchy-graph
Mar 17, 2026
Merged

Unify hierarchy and relationship edges into a single labelled graph#57
mindsocket merged 1 commit into
mainfrom
feat/56-relationships-and-hierarchy-graph

Conversation

@mindsocket
Copy link
Copy Markdown
Owner

Summary

  • Generalises resolveHierarchyEdgesresolveGraphEdges to handle both hierarchy levels and relationship definitions in a single post-parse pass
  • ResolvedParentRef gains source: 'hierarchy' | 'relationship' so downstream consumers can route edges without re-inspecting the schema
  • validate-hierarchyvalidate-graph, with explicit source filtering (hierarchy → structural checks, relationship → ref checks)
  • classifyNodes, show, diagram, Miro sync all filter to source: 'hierarchy' for tree/structural purposes
  • Fixes multimultiple rename in metadata contract and all configured space schemas
  • Docs updated throughout: concepts (unified graph section), schemas reference, schema-authoring, architecture boundary table
  • Repro test replaced with permanent source-aware regression tests

What's still open (tracked in #56 / #54)

Test plan

  • bun run test — 207 tests pass
  • bun run test:smoke — validate all configured spaces (pre-existing failures unaffected)
  • Review resolvedParents output via dump on a space with relationships to confirm source field is correct
  • Verify Miro sync only creates connectors for hierarchy edges (not relationship edges)

Closes part of #56

…raph

Both hierarchy levels and relationships now resolve into a single
`resolvedParents: ResolvedParentRef[]` array on each node. Each ref
carries `source: 'hierarchy' | 'relationship'` so downstream consumers
can route edges without re-inspecting the schema.

Key changes:
- `resolveHierarchyEdges` → `resolveGraphEdges`, generalised to handle
  both hierarchy levels and relationship definitions in one post-parse pass
- `validate-hierarchy` → `validate-graph`, with explicit `source` filtering:
  hierarchy edges go to structural checks, relationship edges to ref checks
- `classifyNodes`, `show`, `diagram` filter to `source: 'hierarchy'`
- Miro sync connector building and cache hashing filter to hierarchy edges
- `multi` → `multiple` rename fixed in metadata contract and all schemas
- `ResolvedParentRef` shape documented; concepts, schemas, architecture,
  and schema-authoring reference updated to reflect the unified graph model
- Repro test (issue #56) replaced with permanent `source`-aware regression
  tests in resolve-hierarchy-edges.test.ts
@claude
Copy link
Copy Markdown

claude Bot commented Mar 17, 2026

Claude finished @mindsocket's task in 1m 34s —— View job


Code Review: PR #57 - Unify hierarchy and relationship edges into a single labelled graph

Overall Assessment: ✅ APPROVED with minor suggestions

This is a well-executed refactoring that unifies the hierarchy and relationship edge resolution into a single, coherent graph model. The architectural decisions are sound, the implementation is clean, and the test coverage is excellent.


✅ Strengths

1. Clean Architecture

  • The source: 'hierarchy' | 'relationship' field on ResolvedParentRef is an elegant solution that lets downstream consumers filter edges without re-inspecting the schema
  • resolveGraphEdges successfully generalizes the previous resolveHierarchyEdges while maintaining clarity
  • The separation of concerns in validateGraph is excellent: hierarchy edges → structural checks, relationship edges → reference checks

2. Comprehensive Test Coverage

  • All 207 tests pass ✅
  • New tests in resolve-hierarchy-edges.test.ts thoroughly cover the source field behavior
  • Tests verify both hierarchy and relationship edge resolution with proper source attribution
  • Edge cases like selfRefField, fieldOn: 'parent', and mixed hierarchies are well-covered

3. Thoughtful Consumer Updates

  • classifyNodes correctly filters to source: 'hierarchy' for tree/structural purposes (src/util/graph-helpers.ts:47)
  • show, diagram commands properly filter to hierarchy edges only
  • Miro sync correctly filters connectors to hierarchy edges (src/integrations/miro/sync.ts:329)
  • Validation command properly routes different edge types

4. Good Documentation Updates

  • docs/concepts.md has a comprehensive new "Graph edges" section explaining the unified model
  • The ResolvedParentRef structure is well-documented with clear field descriptions
  • Architecture and schema-authoring docs updated appropriately

🔍 Issues Found

Minor: Stray Statement


💡 Suggestions (Optional)

1. Type Safety
The source field on ResolvedParentRef is a string union. Consider using a const assertion or enum for better type safety if this pattern grows:

export type EdgeSource = 'hierarchy' | 'relationship';
export interface ResolvedParentRef {
  source: EdgeSource;
  // ...
}

2. Documentation Enhancement
Consider adding a visual diagram to docs/concepts.md showing how hierarchy and relationship edges both flow into the unified resolvedParents array, similar to the existing space parsing diagram.


📊 Test Results

  • Unit Tests: 207/207 passing
  • Coverage: New source field behavior thoroughly tested
  • Edge Cases: selfRefField, fieldOn: 'parent', mixed hierarchies all covered

🎯 Summary

This refactoring successfully unifies the edge resolution model while maintaining backward compatibility for all consumers. The addition of the source field is a clean, extensible solution that enables proper filtering without sacrificing performance or clarity.

Recommendation: Merge after addressing the minor stray statement issue. The overall implementation is solid and ready for production.

@mindsocket mindsocket merged commit ce7313e into main Mar 17, 2026
2 checks passed
@mindsocket mindsocket deleted the feat/56-relationships-and-hierarchy-graph branch March 17, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant