Fix #114: Add FAQ question blocks with configurable section grouping - #122
Conversation
📝 WalkthroughWalkthroughChangesFAQ question blocks now support validated FAQ question blocks
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ContentProcessorPipeline
participant QuestionProcessor
participant MarkdownProcessor
participant HTMLDocument
ContentProcessorPipeline->>QuestionProcessor: preserve question shortcodes
QuestionProcessor->>MarkdownProcessor: pass encoded question markers
MarkdownProcessor->>QuestionProcessor: return Markdown-rendered content
QuestionProcessor->>HTMLDocument: replace markers with FAQ details sections
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Pull request overview
Adds first-class support for ::: question FAQ blocks in YiiPress content, with per-entry configuration (faq_level) to keep questions inline or group them into semantic, no-JS <details>/<summary> FAQ sections. This integrates the processor into both page and feed pipelines prior to TOC collection, and updates the minimal theme, docs, tests, benchmarks, and roadmap accordingly.
Changes:
- Introduce a two-pass
QuestionProcessorthat preserves question blocks pre-Markdown and renders/groups them post-Markdown. - Add
faq_levelparsing to entries (front matter →Entry::$faqLevel) and wire the processor into the DI content pipelines. - Add minimal theme styling, PHPUnit coverage, phpbench benchmark, and user-facing documentation.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| themes/minimal/assets/style.css | Adds minimal theme styling for .faq-section, .faq-question, and .faq-answer. |
| tests/Unit/Theme/MinimalThemeAssetsTest.php | Verifies minimal theme CSS includes FAQ styles. |
| tests/Unit/Processor/QuestionProcessorTest.php | Covers inline rendering, grouping modes, fence edge cases, and code-fence suppression. |
| tests/Unit/Processor/ContentProcessorPipelineConfigTest.php | Ensures QuestionProcessor is registered in the expected pipeline positions (two-pass). |
| tests/Unit/Content/Parser/EntryParserTest.php | Adds parsing/validation coverage for faq_level front matter. |
| src/Processor/Question/QuestionProcessor.php | Implements preservation + post-Markdown rendering/grouping into <details>/<summary>. |
| src/Content/Parser/EntryParser.php | Parses strict faq_level values and throws on invalid configs. |
| src/Content/Model/Entry.php | Adds faqLevel field to the entry model and preserves it in withRedirectTo(). |
| roadmap.md | Checks off issue #114 as completed. |
| docs/plugins.md | Documents ::: question usage, grouping behavior, and the JSON-LD decision. |
| docs/content.md | Documents faq_level in front matter reference. |
| docs/benchmarking.md | Lists QuestionProcessorBench. |
| config/common/di/content-pipeline.php | Inserts QuestionProcessor into page + feed pipelines (two-pass, before TOC). |
| benchmarks/QuestionProcessorBench.php | Adds a 100-question benchmark for inline vs section-grouped processing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n-blocks-with-configurable-section-grouping # Conflicts: # docs/content.md # roadmap.md # src/Content/Model/Entry.php # src/Content/Parser/EntryParser.php # tests/Unit/Content/Parser/EntryParserTest.php
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Processor/Question/QuestionProcessor.php (1)
43-43: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid allocating a lowercase copy for a case-insensitive substring check.
strtolower($content)copies the entire content string just to check for[question. Usestripos()instead to avoid the allocation, especially since this runs on the full rendered document on the second pipeline pass.⚡ Proposed fix
- if (!str_contains(strtolower($content), '[question')) { + if (stripos($content, '[question') === false) { return $content; }🤖 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/Processor/Question/QuestionProcessor.php` at line 43, Update the substring check in QuestionProcessor to use stripos() for a case-insensitive search of “[question” instead of lowercasing $content with strtolower(). Preserve the existing condition’s behavior for both matching and non-matching content.config/common/di/content-pipeline.php (1)
48-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument why
QuestionProcessorruns at each of these two points.The existing comments explain only
CodeGroupProcessor's two-pass placement. Add a short comment noting thatQuestionProcessorfollows the same preserve-before-Markdown, render-after-later-processors pattern. This pipeline is order-sensitive, and a future reordering mistake would silently break FAQ rendering without a compile-time error.📝 Proposed comment additions
// Preserve code-group shortcode metadata before Markdown, then render it after syntax highlighting. Reference::to(CodeGroupProcessor::class), + // Preserve question shortcodes before Markdown; rendered below after the second code-group pass. Reference::to(QuestionProcessor::class), Reference::to(MarkdownProcessor::class), Reference::to(LatexMathProcessor::class), Reference::to(TagLinkProcessor::class), Reference::to(MermaidProcessor::class), Reference::to(SyntaxHighlightProcessor::class), // Complete the second code-group pass using the rendered, highlighted code blocks. Reference::to(CodeGroupProcessor::class), + // Render preserved question shortcodes into FAQ details markup. Reference::to(QuestionProcessor::class), Reference::to(TocProcessor::class),Also applies to: 65-68
🤖 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 `@config/common/di/content-pipeline.php` around lines 48 - 58, Add concise comments at both QuestionProcessor placements in the content pipeline, explaining that its first pass preserves question metadata before Markdown and its second pass renders it after the later processors. Keep the existing CodeGroupProcessor comments and processor ordering unchanged.
🤖 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 `@tests/Unit/Theme/MinimalThemeAssetsTest.php`:
- Around line 89-95: Add PHPUnit assertions in MinimalThemeAssetsTest covering
the remaining FAQ CSS rules: the .faq-question:last-child bottom border,
.faq-answer padding, and the first-child and last-child margin normalization
rules. Keep the existing FAQ style assertions and verify each expected
declaration is present in $css.
---
Nitpick comments:
In `@config/common/di/content-pipeline.php`:
- Around line 48-58: Add concise comments at both QuestionProcessor placements
in the content pipeline, explaining that its first pass preserves question
metadata before Markdown and its second pass renders it after the later
processors. Keep the existing CodeGroupProcessor comments and processor ordering
unchanged.
In `@src/Processor/Question/QuestionProcessor.php`:
- Line 43: Update the substring check in QuestionProcessor to use stripos() for
a case-insensitive search of “[question” instead of lowercasing $content with
strtolower(). Preserve the existing condition’s behavior for both matching and
non-matching content.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c371c47-9ae2-42a9-b40b-f6cf15f85ccb
📒 Files selected for processing (14)
benchmarks/QuestionProcessorBench.phpconfig/common/di/content-pipeline.phpdocs/benchmarking.mddocs/content.mddocs/plugins.mdroadmap.mdsrc/Content/Model/Entry.phpsrc/Content/Parser/EntryParser.phpsrc/Processor/Question/QuestionProcessor.phptests/Unit/Content/Parser/EntryParserTest.phptests/Unit/Processor/ContentProcessorPipelineConfigTest.phptests/Unit/Processor/QuestionProcessorTest.phptests/Unit/Theme/MinimalThemeAssetsTest.phpthemes/minimal/assets/style.css
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Processor/Question/QuestionProcessor.php:46
- The PR description/issue motivation references Rapira-style
::: questioncontainers, but this processor only recognizes[question ...]...[/question]shortcodes (see the[questiondetection and open-tag regex). This is a behavior/documentation mismatch that may confuse users expecting the documented Rapira syntax.
Either update the PR description/docs to explicitly state the supported syntax is [question], or add support for ::: question blocks (including tests and docs) if that syntax is still a requirement.
if (str_contains($content, self::START_MARKER)) {
return $this->renderQuestions($content, $entry->faqLevel);
}
if (stripos($content, '[question') === false) {
return $content;
}
Closes #114.
Summary
faq_levelgrouping configuration::: questioncontainers outside code fencesdetails/summarymarkupValidation
make test CLI_ARGS="tests/Unit/Processor/QuestionProcessorTest.php tests/Unit/Content/Parser/EntryParserTest.php tests/Unit/Processor/ContentProcessorPipelineConfigTest.php tests/Unit/Theme/MinimalThemeAssetsTest.php"(49 tests, 201 assertions)make psalmmake composer-dependency-analysermake bench BENCH_FILTER=QuestionProcessorBenchgit diff --checkBenchmark result in this environment: 100 inline questions ~128.6µs; grouped by section ~152.5µs.
Summary by CodeRabbit
New Features
[question]shortcodes.Style
Documentation