Conversation
WalkthroughFlattens subsections into underscore-separated page names; replaces legacy content-resolution with enriched-collection and content-matching utilities; adds example/import parsing helpers; updates API routes, index generation, tests, docs, configs, and devDependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as API Client
participant Route as API Route
participant Index as ApiIndex
participant Collections as EnrichedCollections
participant Matcher as ContentMatcher
Client->>Route: GET /api/v6/components/forms_checkbox[/tab]
Route->>Index: validate section in index.sections[v6]
Index-->>Route: section present
Route->>Collections: getEnrichedCollections(v6)
Collections-->>Route: EnrichedContentEntry[]
Route->>Matcher: findContentEntry(entries, {section, page, tab})
Matcher-->>Route: matched entry or null
alt matched
Route-->>Client: 200 Content payload
else not found
Route-->>Client: 404 Not found
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
Deploying patternfly-doc-core with
|
| Latest commit: |
d41120e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f11efe1a.patternfly-doc-core.pages.dev |
| Branch Preview URL: | https://add-subsection-support.patternfly-doc-core.pages.dev |
![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
🤖 Fix all issues with AI agents
In `@src/utils/apiRoutes/exampleParsing.ts`:
- Around line 24-37: The current extractExampleFilePath uses imports.find(imp =>
imp.includes(exampleName)) which can match substrings (e.g., AlertBasic vs
AlertBasicExpanded); change the selection to use a word-boundary regex with the
exampleName escaped: build an escapedName by replacing special regex chars,
create a RegExp like new RegExp(`\\b${escapedName}\\b`) and use imports.find(imp
=> wordRegex.test(imp)) to locate exampleImport; keep the existing
path-extraction logic (match on /['"](\.[^'"]+)['"]/i) and ensure the escape
helper is defined in this module.
🧹 Nitpick comments (1)
src/utils/apiRoutes/contentMatching.ts (1)
24-51: Consider extracting the shared matching logic.Both
findContentEntryandfindContentEntryFilePathcontain nearly identical matching logic (lines 30-48 and 70-86): section check, tab comparison withaddDemosOrDeprecated, and page construction with subsection handling.Extract a shared predicate function to reduce duplication and ensure consistent matching behavior.
♻️ Proposed refactor
+function matchesParams(entry: EnrichedContentEntry, params: ContentMatchParams): boolean { + const { section, page, tab } = params + + if (entry.data.section !== section) { + return false + } + + const entryTab = addDemosOrDeprecated(entry.data.tab, entry.filePath) + if (entryTab !== tab) { + return false + } + + const entryId = kebabCase(entry.data.id) + const entryPage = entry.data.subsection + ? `${entry.data.subsection}_${entryId}` + : entryId + + return entryPage === page +} export function findContentEntry( entries: EnrichedContentEntry[], params: ContentMatchParams ): EnrichedContentEntry | null { - const { section, page, tab } = params - - const matchingEntry = entries.find((entry) => { - // Match section and tab - if (entry.data.section !== section) { - return false - } - - const entryTab = addDemosOrDeprecated(entry.data.tab, entry.filePath) - if (entryTab !== tab) { - return false - } - - // Match page (handling flattened subsection names) - const entryId = kebabCase(entry.data.id) - const entryPage = entry.data.subsection - ? `${entry.data.subsection}_${entryId}` - : entryId - - return entryPage === page - }) - - return matchingEntry || null + return entries.find((entry) => matchesParams(entry, params)) || null } export function findContentEntryFilePath( entries: EnrichedContentEntry[], params: ContentMatchParams ): string | null { - const { section, page, tab } = params - - // Find all matching entries - const matchingEntries = entries.filter((entry) => { - if (entry.data.section !== section) { - return false - } - - const entryTab = addDemosOrDeprecated(entry.data.tab, entry.filePath) - if (entryTab !== tab) { - return false - } - - const entryId = kebabCase(entry.data.id) - const entryPage = entry.data.subsection - ? `${entry.data.subsection}_${entryId}` - : entryId - - return entryPage === page - }) + const matchingEntries = entries.filter((entry) => matchesParams(entry, params)) if (matchingEntries.length === 0) { return null } // Prefer .mdx over .md (mdx files contain LiveExample components) const mdxEntry = matchingEntries.find((entry) => entry.filePath.endsWith('.mdx')) const selectedEntry = mdxEntry || matchingEntries[0] return selectedEntry.filePath }Also applies to: 63-97
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/pages/api/openapi.json.ts`:
- Line 342: Replace the invalid multi-option example strings in the OpenAPI
schema with a single valid example value: update the example currently set to
'alert or forms_checkbox' in src/pages/api/openapi.json.ts to a single string
(e.g., 'alert' or 'forms_checkbox') and do the same for the other occurrences in
the same file where example contains "or" (the other schema entries with example
fields at the other noted locations); ensure each schema property uses one
concrete example string rather than multiple options.
In `@src/utils/case.ts`:
- Around line 36-41: The removeSubsection function incorrectly returns only the
segment after the first underscore for multi-underscore page names; update
removeSubsection to return everything after the first underscore instead of just
split('_')[1]—for example, use split('_').slice(1).join('_') or substring from
page.indexOf('_')+1 so names like forms_date_picker yield date_picker; keep the
function name removeSubsection and preserve the original fallback of returning
the whole page when no underscore exists.
🧹 Nitpick comments (3)
src/utils/__tests__/case.test.ts (1)
57-66: Add test for page names with multiple underscores.The current tests don't cover the edge case where the base page name itself contains underscores (e.g.,
forms_date_pickerwheredate_pickeris the actual page name).🧪 Suggested test addition
describe('removeSubsection', () => { it('removes subsection from page name by splitting on underscore', () => { expect(removeSubsection('forms_checkbox')).toBe('checkbox') expect(removeSubsection('components_button')).toBe('button') }) + + it('handles page names with multiple underscores', () => { + expect(removeSubsection('forms_date_picker')).toBe('date_picker') + expect(removeSubsection('menus_multi_select')).toBe('multi_select') + }) it('returns page name when no subsection exists', () => { expect(removeSubsection('alert')).toBe('alert') expect(removeSubsection('button')).toBe('button') }) })src/__tests__/pages/api/__tests__/[version]/[section]/[page]/props.test.ts (1)
41-47: Mock mirrors the production bug.The mock implementation uses the same
split('_')[1]pattern as the actualremoveSubsectionfunction. If the production code is fixed to handle multi-underscore page names, update this mock accordingly.src/pages/api/index.ts (1)
135-165: Use a single valid page ID for the example value.
Theexampleshould be one concrete ID; keep alternatives in the description.♻️ Suggested tweak
- description: 'Page ID (may be underscore-separated for subsection pages like "forms_checkbox")', - example: 'alert or forms_checkbox', + description: 'Page ID (standard or underscore-separated for subsection pages, e.g., "alert" or "forms_checkbox")', + example: 'forms_checkbox',
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/__tests__/case.test.ts`:
- Around line 64-74: Add a test case to cover hyphenated subsection prefixes for
the removeSubsection utility: in the describe('removeSubsection') block add an
expectation that removeSubsection('forms-group_checkbox') returns 'checkbox'
(this verifies the roundtrip with addSubsection-style prefixes like
'forms-group'). Ensure the new assertion sits alongside the existing
underscore-based and no-subsection tests so hyphenated-subsection behavior is
validated.
🧹 Nitpick comments (1)
src/utils/apiRoutes/contentMatching.ts (1)
5-7: Remove unused exported typeContentEntry.The
ContentEntrytype is exported but not imported or used anywhere in the codebase. This unused type should be removed to reduce API surface.
| describe('removeSubsection', () => { | ||
| it('removes subsection from page name by splitting on underscore', () => { | ||
| expect(removeSubsection('forms_checkbox')).toBe('checkbox') | ||
| expect(removeSubsection('components_button')).toBe('button') | ||
| }) | ||
|
|
||
| it('returns page name when no subsection exists', () => { | ||
| expect(removeSubsection('alert')).toBe('alert') | ||
| expect(removeSubsection('button')).toBe('button') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the implementation of removeSubsection
find . -type f \( -name "*.ts" -o -name "*.js" \) ! -path "*/node_modules/*" | head -20Repository: patternfly/patternfly-doc-core
Length of output: 673
🏁 Script executed:
# Search for removeSubsection function implementation
rg -n "removeSubsection\s*=" --type ts --type jsRepository: patternfly/patternfly-doc-core
Length of output: 146
🏁 Script executed:
# Also check the test file to see imports and the actual implementation context
cat -n src/utils/__tests__/case.test.ts | head -100Repository: patternfly/patternfly-doc-core
Length of output: 3457
🏁 Script executed:
# Search for page IDs/names that might contain underscores
rg -n 'id:\s*["\x27]?\w+_\w+|"[a-z]+_[a-z]+"' --type=md --type=mdx --type=json -g '!node_modules' | head -30Repository: patternfly/patternfly-doc-core
Length of output: 101
🏁 Script executed:
# View the implementation of removeSubsection in case.ts
cat -n src/utils/case.ts | sed -n '30,50p'Repository: patternfly/patternfly-doc-core
Length of output: 471
🏁 Script executed:
# View more context around removeSubsection
cat -n src/utils/case.tsRepository: patternfly/patternfly-doc-core
Length of output: 1476
🏁 Script executed:
# Search for page names that might contain underscores in actual content/config
rg -n 'checkbox|button|alert|forms|components' --type ts --type js -A 2 -B 2 | head -100Repository: patternfly/patternfly-doc-core
Length of output: 7564
🏁 Script executed:
# Check how removeSubsection is actually being used in the codebase
rg -n 'removeSubsection' --type ts --type js -B 3 -A 3Repository: patternfly/patternfly-doc-core
Length of output: 5867
🏁 Script executed:
# Verify the split behavior with hyphens to confirm implementation works
python3 << 'EOF'
# Test the split logic
test_cases = [
'forms_checkbox',
'forms-group_checkbox',
'alert'
]
for case in test_cases:
if '_' in case:
result = case.split('_')[1]
else:
result = case
print(f"removeSubsection('{case}') → '{result}'")
EOFRepository: patternfly/patternfly-doc-core
Length of output: 211
Add test coverage for removeSubsection with hyphenated subsections.
The removeSubsection tests only verify simple cases like forms_checkbox. However, addSubsection can produce hyphenated subsection patterns (e.g., forms-group_checkbox when passing 'Forms Group'), so removeSubsection should have a test that verifies this roundtrip behavior.
Add a test case like:
expect(removeSubsection('forms-group_checkbox')).toBe('checkbox')
🤖 Prompt for AI Agents
In `@src/utils/__tests__/case.test.ts` around lines 64 - 74, Add a test case to
cover hyphenated subsection prefixes for the removeSubsection utility: in the
describe('removeSubsection') block add an expectation that
removeSubsection('forms-group_checkbox') returns 'checkbox' (this verifies the
roundtrip with addSubsection-style prefixes like 'forms-group'). Ensure the new
assertion sits alongside the existing underscore-based and no-subsection tests
so hyphenated-subsection behavior is validated.
There was a problem hiding this comment.
@coderabbitai I don't think this is really needed as these different cases would be functionally identical to the function
There was a problem hiding this comment.
@wise-king-sullyman You're absolutely right. Since removeSubsection simply splits on the underscore and returns the second part, it doesn't matter whether the prefix is forms or forms-group—the logic is identical. The existing tests already cover the split behavior adequately. Thanks for the clarification!
🧠 Learnings used
Learnt from: thatblindgeye
Repo: patternfly/patternfly-doc-core PR: 203
File: src/__tests__/utils/tokens.test.ts:26-31
Timestamp: 2026-01-23T20:31:15.852Z
Learning: In PatternFly (patternfly/react-tokens), some tokens don't have a version prefix (e.g., v6) in their name and var properties. For example, both `--pf-v6-t-global--color--100` and `--pf-t-global--color--200` are valid token structures. This variation is intentional and should be accommodated in tests and implementations.
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
Reapproving with the update for the extensions subsection issue
|
🎉 This PR is included in version 1.19.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
Closes #187
Assisted by Claude Code
Summary by CodeRabbit
Documentation
New Features
Refactor
Tests
Chores