feat(dogfood): expand block-authored surfaces#116
Conversation
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 59 minutes and 31 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR implements DF-071 DOGFOOD Block-Authored Surfaces by introducing six new semantic block contracts (search panel, notification center, performance HUD, help overlay, command palette, footer hints), integrating them throughout the docs app with rendering helpers and an auto-generated surface inventory guide, updating tests and i18n baselines, and fixing frame surface painting to prevent terminal background colors from leaking into Bijou-owned surfaces. ChangesDOGFOOD Block-Authored Surfaces
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 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)
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)
examples/docs/dogfood-blocks.ts (1)
1794-1824: ⚡ Quick winHonor the title-only fast path in all modes.
Line 1794 adds
title, but Line 1806-1808 only applies that branch inpipe/accessible. Ininteractive/static, the same title-only config falls back to debug-style output, which can break search chrome title routing.💡 Proposed patch
function renderSearchPanelBlock( input: BlockRenderInput<SearchPanelBlockConfig>, ): BlockRenderResult<string> { const title = input.config?.title; const query = input.config?.query ?? ''; const resultCount = input.config?.resultCount ?? 0; const activeResultLabel = input.config?.activeResultLabel ?? 'none'; const queryLabel = query.trim() === '' ? 'empty' : query; const titleOnly = title != null && query.trim() === '' && resultCount === 0 && activeResultLabel === 'none'; + const facts = [ + { kind: 'entity' as const, key: 'dogfood.block', value: 'SearchPanelBlock' }, + { kind: 'state' as const, key: 'dogfood.search.resultCount', value: String(resultCount) }, + ]; + + if (titleOnly) { + return { + output: title, + facts, + }; + } if (input.mode === 'pipe' || input.mode === 'accessible') { return { - output: titleOnly - ? title - : `Search query: ${queryLabel}; results: ${resultCount}; active: ${activeResultLabel}`, - facts: [ - { kind: 'entity', key: 'dogfood.block', value: 'SearchPanelBlock' }, - { kind: 'state', key: 'dogfood.search.resultCount', value: String(resultCount) }, - ], + output: `Search query: ${queryLabel}; results: ${resultCount}; active: ${activeResultLabel}`, + facts, }; } return { @@ - facts: [ - { kind: 'entity', key: 'dogfood.block', value: 'SearchPanelBlock' }, - { kind: 'state', key: 'dogfood.search.resultCount', value: String(resultCount) }, - ], + facts, }; }🤖 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 `@examples/docs/dogfood-blocks.ts` around lines 1794 - 1824, The title-only fast path (variable titleOnly) is only honored for input.mode 'pipe'/'accessible' but must apply in all modes; update the logic in SearchPanelBlock handling so that when titleOnly is true you return the title string and the same facts regardless of input.mode (i.e., check titleOnly before/independent of the input.mode branch or add the same title-only conditional to the final return), referencing the titleOnly variable and the output/facts structures so interactive/static modes do not fall back to debug-style output.
🤖 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 `@examples/docs/dogfood-blocks.ts`:
- Around line 1837-1840: The returned output string for the pipe/accessible
branch uses the incorrect phrase "Notifications items"; update the string in the
block that checks input.mode === 'pipe' || input.mode === 'accessible' so it
reads "Notification items: ${notificationCount}; filter: ${activeFilterLabel}"
(change "Notifications" to singular "Notification" in the output property).
---
Nitpick comments:
In `@examples/docs/dogfood-blocks.ts`:
- Around line 1794-1824: The title-only fast path (variable titleOnly) is only
honored for input.mode 'pipe'/'accessible' but must apply in all modes; update
the logic in SearchPanelBlock handling so that when titleOnly is true you return
the title string and the same facts regardless of input.mode (i.e., check
titleOnly before/independent of the input.mode branch or add the same title-only
conditional to the final return), referencing the titleOnly variable and the
output/facts structures so interactive/static modes do not fall back to
debug-style output.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7aab30e7-44d0-4a19-b653-98bd7cbd0e6f
📒 Files selected for processing (10)
docs/BEARING.mddocs/CHANGELOG.mddocs/design/DF-071-dogfood-block-authored-surfaces.mddocs/legends/DF-dogfood-field-guide.mdexamples/docs/app.tsexamples/docs/dogfood-blocks.tsexamples/docs/i18n-debt.tstests/cycles/DF-069/dogfood-block-registry.test.tstests/cycles/DF-071/dogfood-block-authored-surfaces.test.tstests/cycles/DX-031/dogfood-blocks-section.test.ts
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)
examples/docs/app.ts (1)
3975-3975:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the guide-page search title template.
Hardcoding
Search ${...}here bakes English grammar and casing into the UI, andtoLowerCase()can mangle localized page titles. Please route this throughdogfoodText(...)with a{pageTitle}placeholder and pass the localized page title through unchanged.Suggested direction
- searchTitle: () => renderDocsSearchTitle(`Search ${pageTitle(spec.id, localization).toLowerCase()}`), + searchTitle: () => renderDocsSearchTitle(dogfoodText( + localization, + 'docs.search.pageTitle', + 'Search {pageTitle}', + { pageTitle: pageTitle(spec.id, localization) }, + )),Also add the matching catalog entry/baseline update.
🤖 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 `@examples/docs/app.ts` at line 3975, Replace the hardcoded English search template in the searchTitle assignment: stop using string interpolation with "Search ${...}" and remove the .toLowerCase() call on pageTitle; instead call dogfoodText with a template key that includes a {pageTitle} placeholder and pass pageTitle(spec.id, localization) as the placeholder value, then feed that result into renderDocsSearchTitle (i.e., use dogfoodText("some.key.searchGuidePage", { pageTitle: pageTitle(spec.id, localization) }) and pass that to renderDocsSearchTitle). Update the related catalog entry/baseline to add the new localization key and baseline text for the search title.
🧹 Nitpick comments (1)
scripts/docs-preview.test.ts (1)
210-218: 💤 Low valueConsider clarifying the test name to match its scope.
The test name suggests it validates "landing title cells" specifically, but the implementation checks every cell in the entire landing frame. While this comprehensive validation aligns well with the PR's frame surface painting fixes, a more precise name would improve clarity for future maintainers.
Consider renaming to something like:
'ensures all cells in the landing frame have Bijou-owned backgrounds''keeps every landing frame cell on a Bijou-owned background'🤖 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 `@scripts/docs-preview.test.ts` around lines 210 - 218, The test name it('keeps every landing title cell on a Bijou-owned background', ...) is misleading because the body checks all cells on the landing frame; rename the test title to match the scope (e.g. "ensures all cells in the landing frame have Bijou-owned backgrounds" or "keeps every landing frame cell on a Bijou-owned background") while leaving the implementation using createTestContext, createDocsApp, runScript, and the assertion against cellsWithoutBackground(frame) unchanged so future maintainers can immediately understand the test's intent.
🤖 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 `@examples/docs/app.ts`:
- Line 3975: Replace the hardcoded English search template in the searchTitle
assignment: stop using string interpolation with "Search ${...}" and remove the
.toLowerCase() call on pageTitle; instead call dogfoodText with a template key
that includes a {pageTitle} placeholder and pass pageTitle(spec.id,
localization) as the placeholder value, then feed that result into
renderDocsSearchTitle (i.e., use dogfoodText("some.key.searchGuidePage", {
pageTitle: pageTitle(spec.id, localization) }) and pass that to
renderDocsSearchTitle). Update the related catalog entry/baseline to add the new
localization key and baseline text for the search title.
---
Nitpick comments:
In `@scripts/docs-preview.test.ts`:
- Around line 210-218: The test name it('keeps every landing title cell on a
Bijou-owned background', ...) is misleading because the body checks all cells on
the landing frame; rename the test title to match the scope (e.g. "ensures all
cells in the landing frame have Bijou-owned backgrounds" or "keeps every landing
frame cell on a Bijou-owned background") while leaving the implementation using
createTestContext, createDocsApp, runScript, and the assertion against
cellsWithoutBackground(frame) unchanged so future maintainers can immediately
understand the test's intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66c9d00d-7b8e-4a24-a051-9ae8c17d69e4
📒 Files selected for processing (6)
docs/CHANGELOG.mdexamples/docs/app.tsexamples/docs/dogfood-blocks.tsscripts/docs-preview.test.tstests/cycles/DF-069/dogfood-block-registry.test.tstests/cycles/DF-071/dogfood-block-authored-surfaces.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/CHANGELOG.md
|
@codex Self-review findings for this branch:
Please confirm whether this should be fixed on this PR before merge. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 750130bf5b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex Addressed the self-review P2 localization ratchet finding in 9420a4b. The DOGFOOD surface block inventory page now resolves its title, summary, body copy, headings, and detail labels through the DOGFOOD string table/runtime catalogs. Added a docs-preview regression that renders the page with a pseudo-localized DOGFOOD catalog, regenerated locale catalogs, and lowered the i18n debt baseline back to the actual count (1993 total / 299 docs-app). Validation: focused docs-preview regression, full docs-preview suite, dogfood i18n debt/export tests, dogfood:i18n:check, dogfood:i18n:debt, typecheck:test, docs:inventory, git diff --check, lint, and pre-push full npm test + interactive example smoke. |
Summary
Testing
Committed on current branch:
I also created the PR record via |
|
Summary
Testing
Committed on current branch:
Also completed per instructions:
|
|
@codex Addressed the DOGFOOD surface inventory localization gap in
Validation after the fix:
✅ Addressed in commits: |
|
Acknowledged — the trigger indicates the localization gap was already fixed in commit I verified the branch state with:
No new code changes were needed, so I did not create a new commit and did not call |
1 participant
Summary
Expands DOGFOOD's block-authored product surface coverage after DF-070.
This stack adds local DOGFOOD block contracts for frame-adjacent surfaces, routes selected visible DOGFOOD chrome through those contracts, and publishes a registry-backed DOGFOOD Surface Blocks page so the app can inspect which product surfaces are now block-authored.
What changed
Architectural boundary
These blocks are semantic DOGFOOD product contracts, not an AppFrame renderer fork. Generic overlay rendering still belongs to
bijou-tui; DOGFOOD owns declared block identity, data requirements, command intents, and inspection inventory.Non-goals
Validation
npm test -- --run tests/cycles/DF-069/dogfood-block-registry.test.ts tests/cycles/DF-071/dogfood-block-authored-surfaces.test.tsnpm run docs:inventorynpm run typecheck:testgit diff --checknpm run lintnpm test— 313 files, 3526 tests passednpm run verify:interactive-examplesnpm run typecheck:test, fullnpm test, andnpm run verify:interactive-examplesSummary by CodeRabbit
New Features
Bug Fixes
Documentation