Headless crawl paths are not exportable#1721
Conversation
WalkthroughChangesHeadless crawling now represents replayable navigation paths, derives them from crawl graphs, exposes them through Headless crawl paths
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Crawler
participant CrawlGraph
participant Path
participant Diagnostics
Crawler->>CrawlGraph: AllPathsFromRoot()
CrawlGraph->>Path: build paths from root
Path-->>CrawlGraph: ordered navigation paths
CrawlGraph-->>Crawler: path collection
Crawler->>Diagnostics: marshal and write paths.json
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. 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)
pkg/engine/headless/types/path.go (1)
41-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrevent nil panics and avoid duplicating struct fields.
If
json.Marshalis invoked on a nil*Path, it will panic when accessingp.Steps. Additionally, mirroring thePathstruct inside the custom marshaller risks omitting new fields added in the future.You can handle both issues robustly by adding a nil check and using a type alias. Embedding a type alias allows you to inherit all original fields and tags, while shadowing
Stepsto guarantee the JSON output is an empty array instead of being dropped byomitempty.♻️ Proposed refactor
// MarshalJSON ensures nil Steps serialize as [] not null. func (p *Path) MarshalJSON() ([]byte, error) { + if p == nil { + return []byte("null"), nil + } steps := p.Steps if steps == nil { steps = []*Action{} } + type Alias Path return json.Marshal(&struct { - EntryID string `json:"entry_id"` - TargetID string `json:"target_id"` - TargetURL string `json:"target_url,omitempty"` + *Alias Steps []*Action `json:"steps"` }{ - EntryID: p.EntryID, - TargetID: p.TargetID, - TargetURL: p.TargetURL, + Alias: (*Alias)(p), Steps: steps, }) }🤖 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 `@pkg/engine/headless/types/path.go` around lines 41 - 58, Update Path.MarshalJSON to return the JSON null representation when the receiver is nil before dereferencing it. Replace the duplicated anonymous struct with a type alias of Path, embedding that alias alongside a shadowed Steps field so all Path fields and tags are preserved while nil Steps serialize as an empty array.
🤖 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 `@pkg/engine/headless/graph/graph.go`:
- Around line 190-215: Update AllPathsFromRoot to construct each path inline
using the already resolved entryID and vertex data, rather than calling
PathFromRoot for every vertex. Reuse the existing ShortestPath logic while
preserving the current root, about:blank, and error-skipping behavior, and avoid
repeatedly invoking RootID or fetching the vertex list during path construction.
---
Nitpick comments:
In `@pkg/engine/headless/types/path.go`:
- Around line 41-58: Update Path.MarshalJSON to return the JSON null
representation when the receiver is nil before dereferencing it. Replace the
duplicated anonymous struct with a type alias of Path, embedding that alias
alongside a shadowed Steps field so all Path fields and tags are preserved while
nil Steps serialize as an empty array.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 189bef6f-71e4-43ce-961a-7c1688ce9e3b
📒 Files selected for processing (6)
pkg/engine/headless/crawler/crawler.gopkg/engine/headless/crawler/paths_test.gopkg/engine/headless/graph/graph.gopkg/engine/headless/graph/graph_test.gopkg/engine/headless/types/path.gopkg/engine/headless/types/path_test.go
| // AllPathsFromRoot returns a path from the entrypoint to every non-entry vertex. | ||
| func (g *CrawlGraph) AllPathsFromRoot() ([]*types.Path, error) { | ||
| entryID, err := g.RootID() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| var paths []*types.Path | ||
| for _, id := range g.GetVertices() { | ||
| if id == entryID { | ||
| continue | ||
| } | ||
| ps, err := g.GetPageState(id) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| if ps.URL == "about:blank" { | ||
| continue | ||
| } | ||
| p, err := g.PathFromRoot(id) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| paths = append(paths, p) | ||
| } | ||
| return paths, nil | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Inline path construction to prevent O(V^2) allocations.
Calling PathFromRoot(id) recalculates RootID() for every vertex. Since RootID() invokes GetVertices() (which fetches the graph's AdjacencyMap and allocates slices), iterating over all vertices results in O(V^2) operations and allocations.
Since AllPathsFromRoot already skips the root vertex and resolves the target's PageState, you can inline ShortestPath to eliminate the redundant overhead.
⚡ Proposed performance optimization
// AllPathsFromRoot returns a path from the entrypoint to every non-entry vertex.
func (g *CrawlGraph) AllPathsFromRoot() ([]*types.Path, error) {
entryID, err := g.RootID()
if err != nil {
return nil, err
}
var paths []*types.Path
for _, id := range g.GetVertices() {
if id == entryID {
continue
}
ps, err := g.GetPageState(id)
if err != nil {
continue
}
if ps.URL == "about:blank" {
continue
}
- p, err := g.PathFromRoot(id)
+ steps, err := g.ShortestPath(entryID, id)
if err != nil {
continue
}
- paths = append(paths, p)
+ paths = append(paths, &types.Path{
+ EntryID: entryID,
+ TargetID: id,
+ TargetURL: ps.URL,
+ Steps: steps,
+ })
}
return paths, nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // AllPathsFromRoot returns a path from the entrypoint to every non-entry vertex. | |
| func (g *CrawlGraph) AllPathsFromRoot() ([]*types.Path, error) { | |
| entryID, err := g.RootID() | |
| if err != nil { | |
| return nil, err | |
| } | |
| var paths []*types.Path | |
| for _, id := range g.GetVertices() { | |
| if id == entryID { | |
| continue | |
| } | |
| ps, err := g.GetPageState(id) | |
| if err != nil { | |
| continue | |
| } | |
| if ps.URL == "about:blank" { | |
| continue | |
| } | |
| p, err := g.PathFromRoot(id) | |
| if err != nil { | |
| continue | |
| } | |
| paths = append(paths, p) | |
| } | |
| return paths, nil | |
| } | |
| // AllPathsFromRoot returns a path from the entrypoint to every non-entry vertex. | |
| func (g *CrawlGraph) AllPathsFromRoot() ([]*types.Path, error) { | |
| entryID, err := g.RootID() | |
| if err != nil { | |
| return nil, err | |
| } | |
| var paths []*types.Path | |
| for _, id := range g.GetVertices() { | |
| if id == entryID { | |
| continue | |
| } | |
| ps, err := g.GetPageState(id) | |
| if err != nil { | |
| continue | |
| } | |
| if ps.URL == "about:blank" { | |
| continue | |
| } | |
| steps, err := g.ShortestPath(entryID, id) | |
| if err != nil { | |
| continue | |
| } | |
| paths = append(paths, &types.Path{ | |
| EntryID: entryID, | |
| TargetID: id, | |
| TargetURL: ps.URL, | |
| Steps: steps, | |
| }) | |
| } | |
| return paths, nil | |
| } |
🤖 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 `@pkg/engine/headless/graph/graph.go` around lines 190 - 215, Update
AllPathsFromRoot to construct each path inline using the already resolved
entryID and vertex data, rather than calling PathFromRoot for every vertex.
Reuse the existing ShortestPath logic while preserving the current root,
about:blank, and error-skipping behavior, and avoid repeatedly invoking RootID
or fetching the vertex list during path construction.
1 participant
Fixes #1715
Pure headless builds a crawl graph but does not expose a replayable path from the entrypoint to each location. This adds a Path type, PathFromRoot / AllPathsFromRoot, Paths() on the crawler, and paths.json when diagnostics are on.
Summary by CodeRabbit
New Features
paths.json.Tests