Derive connector docs URLs from the sitemap instead of a hardcoded Area-to-category map - #48
Conversation
…ea-to-category map
📝 WalkthroughSummary
WalkthroughConnector documentation resolution now uses hardcoded package mappings or canonical overview URLs derived from the documentation sitemap. The sitemap-derived map is cached in Sequence Diagram(s)sequenceDiagram
participant ConnectorDetailPage
participant ConnectorUtils
participant DocsSitemap
participant localStorage
ConnectorDetailPage->>ConnectorUtils: request documentation URL map
ConnectorUtils->>localStorage: read cached map
ConnectorUtils->>DocsSitemap: fetch sitemap on cache miss
DocsSitemap-->>ConnectorUtils: sitemap URLs
ConnectorUtils->>localStorage: cache canonical overview URLs
ConnectorUtils-->>ConnectorDetailPage: package-to-URL map
ConnectorDetailPage->>ConnectorUtils: resolve package override
ConnectorUtils-->>ConnectorDetailPage: documentation URL or undefined
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 2
🤖 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 `@src/lib/connector-utils/connector-utils.ts`:
- Around line 557-558: Version the SITEMAP_CACHE_KEY used by the sitemap cache
so pre-deploy Set-shaped entries cannot be read as the new {entries, timestamp}
format. Update the cache read logic around the sitemap caching flow to trust
parsed data only when entries is an array, otherwise treat it as a cache miss
and rebuild the cache.
- Around line 592-601: Update getConnectorDocsUrlMap so sitemapPromise stores
and returns the promise with the empty-map rejection fallback applied, rather
than retaining the raw async IIFE promise. Ensure concurrent callers hitting the
existing sitemapPromise guard receive the same resolved empty Map when fetch or
parsing fails, including the corresponding handling near the later
sitemapPromise assignment.
🪄 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 Plus
Run ID: 8153a57d-b8e1-4cc4-aa29-15fe0350e2d4
📒 Files selected for processing (3)
src/lib/connector-utils/connector-utils.tssrc/lib/connector-utils/index.tssrc/pages/ConnectorDetailPage.tsx
| const SITEMAP_CACHE_KEY = 'connector_docs_sitemap'; | ||
| const SITEMAP_CACHE_TTL = 6 * 60 * 60 * 1000; // 6 hours |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Cache key reuse across schema change risks stale-format reads.
SITEMAP_CACHE_KEY is unchanged from the prior Set-based caching scheme, but the stored shape moved to {entries, timestamp}. If a client's browser already holds a pre-deploy cache entry under this key, entries would be undefined after parsing, and new Map(undefined) silently yields an empty map that is then treated as a fresh, valid cache for the remaining TTL window — degrading documentation links to the fallback state for up to 6 hours post-deploy.
Consider bumping the cache key (e.g. append a version suffix) or validating Array.isArray(entries) before trusting the cached value.
Also applies to: 597-602
🤖 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 `@src/lib/connector-utils/connector-utils.ts` around lines 557 - 558, Version
the SITEMAP_CACHE_KEY used by the sitemap cache so pre-deploy Set-shaped entries
cannot be read as the new {entries, timestamp} format. Update the cache read
logic around the sitemap caching flow to trust parsed data only when entries is
an array, otherwise treat it as a cache miss and rebuild the cache.
…nt-caller fallback
|
Addressed both CodeRabbit findings in f1592ef:
|
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)
src/lib/connector-utils/connector-utils.ts (1)
623-633: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCheck the sitemap response before caching its contents.
A non-success sitemap response still reaches
response.text()and can generate an empty/partial map that is persisted for the 6-hour TTL. Checkresponse.okbefore parsing and throw so the outer fallback returns an empty map without caching bad data.🔧 Proposed fix
const response = await fetch(DOCS_SITEMAP_URL); +if (!response.ok) { + throw new Error(`Failed to fetch connector sitemap: ${response.status}`); +} const xml = await response.text();🤖 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 `@src/lib/connector-utils/connector-utils.ts` around lines 623 - 633, Update the sitemap-fetching flow before response.text() to validate response.ok and throw on non-success responses. Ensure the existing outer fallback handles the error by returning an empty map, preventing the docsUrlMap from being persisted to localStorage when the response is invalid.
🤖 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 `@src/lib/connector-utils/connector-utils.ts`:
- Around line 623-633: Update the sitemap-fetching flow before response.text()
to validate response.ok and throw on non-success responses. Ensure the existing
outer fallback handles the error by returning an empty map, preventing the
docsUrlMap from being persisted to localStorage when the response is invalid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fd3629d5-a775-4d2d-b7c9-966db0c9da72
📒 Files selected for processing (1)
src/lib/connector-utils/connector-utils.ts
2 participants
Summary
getConnectorDocsUrlMap()(renamed fromgetDocumentedConnectors()) now parses the sitemap into a full package name -> docs URL map, picking the "overview" page when a package has multiple docs pages.CONNECTOR_DOCSandgetConnectorDocsUrl()are kept only as a manual override for the rare cases the sitemap can't resolve.ConnectorDetailPagechecks the manual override first, then falls back to the sitemap-derived map.Test plan
npm run typecheckpasses