fix: render inter-domain relationships in graph2md#7
Open
jonathanpopham wants to merge 2 commits intomainfrom
Open
fix: render inter-domain relationships in graph2md#7jonathanpopham wants to merge 2 commits intomainfrom
jonathanpopham wants to merge 2 commits intomainfrom
Conversation
Domain entity pages were missing connections between domains because the relationship switch block silently dropped domain-to-domain edges. Add domainEdge struct and domainRelatesOut/domainRelatesIn index maps, populate them via a default case for any relationship connecting two Domain nodes, and render them in writeGraphData, writeMermaidDiagram, and writeDomainBody. Closes #6
WalkthroughAdds capture and rendering of domain-to-domain relationships: parses inter-domain edges into new indexes, threads them through renderContext.Run, and includes related-domain info in domain body text, graph_data frontmatter, and Mermaid diagrams. Changes
Sequence Diagram(s)sequenceDiagram
participant Parser as Graph Parser
participant Indexer as Domain Indexer
participant Renderer as Render Engine
participant Writer as File Writer
Parser->>Indexer: parse nodes & relationships
Indexer-->>Indexer: build domainRelatesOut/in maps
Indexer->>Renderer: pass nodes, relationships, domainRelates maps
Renderer->>Renderer: generate domain body, graph_data, mermaid (including relatesTo edges)
Renderer->>Writer: emit markdown files with frontmatter and diagrams
Writer-->>Renderer: write success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/graph2md/graph2md_test.go (1)
56-101: Consider populatingdomainNodeByNamefor completeness.Right now the helper sets up
nodeLookupandslugLookup, butdomainNodeByNamestays empty (line 93). This is fine for the current tests, but if you later add a test that exercisesdomainLink()(which looks up domains by name), it'll return plain text instead of a link.Quick fix if you ever need it:
♻️ Suggested enhancement
for i := range allNodes { n := &allNodes[i] nodeLookup[n.ID] = n slugLookup[n.ID] = toSlug(getStr(n.Properties, "name")) + if hasLabel(n, "Domain") { + // Populate domainNodeByName so domainLink() works + domainNodeByName[getStr(n.Properties, "name")] = n.ID + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/graph2md/graph2md_test.go` around lines 56 - 101, The helper domainRenderCtx builds nodeLookup and slugLookup but leaves domainNodeByName empty; update domainRenderCtx to populate domainNodeByName by iterating allNodes (and center) and mapping each node's name (getStr(node.Properties,"name")) or slug to the node ID using the same toSlug logic so domainNodeByName[nameOrSlug] = node.ID; ensure you handle center the same way as other nodes and use the existing toSlug/getStr helpers so domainLink() can resolve names to links.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/graph2md/graph2md_test.go`:
- Around line 407-424: The extractGraphDataJSON test helper can panic when the
closing quote pattern isn't found; change the logic in extractGraphDataJSON to
compute endIdx := strings.Index(rest, "\"\n") and if endIdx == -1 call t.Fatalf
with a clear message (including the offending frontmatter or rest) instead of
slicing with -1, then build full using rest[:endIdx] as before and proceed with
json.Unmarshal; this prevents a slice bounds panic and gives a helpful test
failure message.
---
Nitpick comments:
In `@internal/graph2md/graph2md_test.go`:
- Around line 56-101: The helper domainRenderCtx builds nodeLookup and
slugLookup but leaves domainNodeByName empty; update domainRenderCtx to populate
domainNodeByName by iterating allNodes (and center) and mapping each node's name
(getStr(node.Properties,"name")) or slug to the node ID using the same toSlug
logic so domainNodeByName[nameOrSlug] = node.ID; ensure you handle center the
same way as other nodes and use the existing toSlug/getStr helpers so
domainLink() can resolve names to links.
- Guard against panic in extractGraphDataJSON when closing quote not found - Populate domainNodeByName in test helper so domainLink() resolves correctly
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/graph2md/graph2md_test.go (1)
23-31: Unused helper functionmakeSubdomainNode.Hey, heads up — this helper is defined but never actually called anywhere in the test file. It's like buying ingredients for a recipe you never cooked.
If you're planning to add subdomain tests later, totally fine to keep it around. Otherwise, consider removing it to keep things tidy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/graph2md/graph2md_test.go` around lines 23 - 31, The helper function makeSubdomainNode is defined but never used; either remove the unused function to tidy the test file or, if you intend to test subdomains, add tests that call makeSubdomainNode (referencing the makeSubdomainNode function and the Node type) so it is exercised; pick one and apply the change consistently in graph2md_test.go.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/graph2md/graph2md_test.go`:
- Around line 23-31: The helper function makeSubdomainNode is defined but never
used; either remove the unused function to tidy the test file or, if you intend
to test subdomains, add tests that call makeSubdomainNode (referencing the
makeSubdomainNode function and the Node type) so it is exercised; pick one and
apply the change consistently in graph2md_test.go.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
graph2mdsilently dropped domain-to-domain relationshipsdomainEdgestruct anddomainRelatesOut/domainRelatesInindex maps that capture any relationship connecting two Domain nodeswriteGraphData(),writeMermaidDiagram(), andwriteDomainBody()to render outgoing and incoming domain relationshipsgraph2md_test.go) with 7 tests including end-to-endTest plan
go test ./...— all 7 tests passgo vet ./...— cleanCloses #6
Summary by CodeRabbit
New Features
Improvements
Tests
Note
Medium Risk
Changes markdown/frontmatter generation for
Domainpages by indexing previously-ignored relationships and emitting new edges/sections, which can affect site output and any consumers ofgraph_data/Mermaid diagrams. Test coverage is added, but behavior changes may surface formatting or size-limit edge cases on large graphs.Overview
graph2mdnow indexes any relationship between twoDomainnodes (captured asdomainRelatesOut/domainRelatesIn) instead of silently dropping them.Domain pages render these connections in three places: a new Related Domains section in the body (with relationship type and incoming/outgoing labeling), additional
relatesToedges ingraph_datafrontmatter, and labeled arrows in the Mermaid domain diagram.Adds a new
graph2md_test.gosuite with unit and end-to-end tests asserting relationship indexing and the new rendered outputs.Written by Cursor Bugbot for commit 33ef5a4. This will update automatically on new commits. Configure here.