fix(import): RM-17387 Sitemap fallback creates sparse imports#30
Conversation
xavierandueza
left a comment
There was a problem hiding this comment.
Code-quality pass: the behavior direction makes sense, but I think the sitemap fallback needs a cleaner shape before this lands. The current implementation adds more serial orchestration and duplicates the docs-subtree narrowing model instead of extracting the shared policy.
| const seenUrls = new Set() | ||
|
|
||
| for (const candidate of buildSitemapCandidates(sourceUrl)) { | ||
| const res = await fetchSitemap(candidate.url) |
There was a problem hiding this comment.
This changes sitemap fallback from "try candidates until one works" into "fetch every candidate", but keeps the network work strictly sequential. These probes are independent, and this path now includes every well-known docs route plus the source-path walk-up; on a site where most candidates 404/timeout, import latency becomes the sum of all failures.
Can we restructure this as bounded/parallel discovery that preserves candidate order when aggregating results? That would make the new gather-first model explicit without turning fallback into a long serial chain. The existing llms route probing already uses Promise.all, so keeping sitemap discovery serial feels like avoidable orchestration complexity rather than a necessary control flow.
| }) | ||
| if (!rootHit) return null | ||
|
|
||
| const firstSegs = new Set() |
There was a problem hiding this comment.
This duplicates the same docs-root segment detection/filtering policy that narrowToDocsSubtreeIfNeeded implements for llms.txt above. The two flows now need to stay manually aligned on: host docs exemption, root-hit gating, first-segment extraction, DOCS_LIKE_ROOT_SEGMENTS, prefix matching, and malformed URL handling.
I think the code-judo move here is to extract the policy, not reimplement it for a second input shape. For example: normalize llms items and sitemap URLs into a simple URL list, run a shared findDocsLikeRootSegment(...) / filterUrlsToRootSegment(...), then let each caller adapt the result back to its shape. That deletes a lot of duplicated branching and keeps future scope tweaks from drifting between llms and sitemap fallback.
🧰 Changes
/learnto produce a one-page skeleton./learnare no longer treated as strong docs-only scopes.🧬 QA & Testing
node --test src/commands/import.test.jsnpm test[RECORDING/IMAGES]