fix(service): parse enriched six-column /graph.md - #62
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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/core/service/hub_test.go (1)
84-86: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the actual node set, not only its length.
A count of two can still pass if one real node is dropped and an edge row replaces it. Compare both expected
hubNodevalues, including their refs and statuses, to fully prove that edge rows are excluded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/core/service/hub_test.go` around lines 84 - 86, Update the node assertions in the relevant hub test to compare g.nodes against the two expected hubNode values, including each node’s refs and statuses, rather than checking only len(g.nodes). Preserve the verification that edge rows are excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/core/service/hub.go`:
- Line 10: Remove the graphstore import and direct graphstore.ParseExport usage
from the core service in hub.go. Define or use a core-owned outbound port and
DTOs for graph export parsing, inject that port into the relevant hub service,
and move the concrete parsing implementation into the graphstore adapter while
preserving existing behavior.
---
Nitpick comments:
In `@internal/core/service/hub_test.go`:
- Around line 84-86: Update the node assertions in the relevant hub test to
compare g.nodes against the two expected hubNode values, including each node’s
refs and statuses, rather than checking only len(g.nodes). Preserve the
verification that edge rows are excluded.
🪄 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 Plus
Run ID: 0350139b-66c2-4762-a988-ec35a1906ff6
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modinternal/core/service/hub.gointernal/core/service/hub_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/core/port/port.go`:
- Around line 198-218: Replace the raw GraphExportNode/GraphExportEdge types and
string-based ParseGraphExport contract in GraphExportParser with domain-owned
references and edges. Move graph.md decoding, mark-endpoint filtering, and URL
normalization into the adapter, then update the hub service to consume the
already-normalized domain values without parsing client-specific strings. Ensure
all ports under internal/core/port use only domain types.
🪄 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 Plus
Run ID: 5bf19711-f9e9-484c-a069-d1c5d967a915
📒 Files selected for processing (8)
cmd/demarkus-library/main.gointernal/adapter/outbound/graphexport/graphexport.gointernal/core/port/port.gointernal/core/service/floor_test.gointernal/core/service/hub.gointernal/core/service/hub_test.gointernal/core/service/reading.gointernal/core/service/worldmap_test.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/core/service/hub_test.go (1)
7-7: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep
internal/core/service/hub_test.goport-only. Replacegraphexport.Parser{}with a localport.GraphExportParserstub here; keep the concrete adapter wiring in adapter/composition tests. The parser behavior already has coverage ininternal/adapter/outbound/graphexport/graphexport_test.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/core/service/hub_test.go` at line 7, Keep hub_test.go focused on core ports by replacing the concrete graphexport.Parser usage with a local stub implementing port.GraphExportParser. Remove the graphexport import and retain concrete parser wiring only in adapter/composition tests; do not duplicate parser behavior coverage here.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/core/service/hub_test.go`:
- Line 7: Keep hub_test.go focused on core ports by replacing the concrete
graphexport.Parser usage with a local stub implementing port.GraphExportParser.
Remove the graphexport import and retain concrete parser wiring only in
adapter/composition tests; do not duplicate parser behavior coverage here.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: abe0dd7a-bcf3-4430-9bd2-693787422376
📒 Files selected for processing (7)
internal/adapter/outbound/graphexport/graphexport.gointernal/adapter/outbound/graphexport/graphexport_test.gointernal/core/domain/document.gointernal/core/port/port.gointernal/core/service/hub.gointernal/core/service/hub_test.gointernal/core/service/worldmap_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/core/service/worldmap_test.go
1 participant
Summary by CodeRabbit
New Features
Bug Fixes