feat(dogfood): polish block-authored docs surfaces#115
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR implements DF-070, making DOGFOOD feel like a block-authored terminal product. It introduces prose-wrapped text rendering to bijou-tui, extends DOGFOOD block contracts, adds locale persistence, routes documentation panes through block renderers, expands i18n coverage, and provides comprehensive test coverage validating the new behavior. ChangesDF-070 DOGFOOD Block Product Polish
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![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: 798185b782
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/docs/dogfood-blocks.ts (1)
1144-1152:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve
headingCountfacts whenbodyis absent.
DocumentationArticleBlockConfig.bodyis optional now, but this fallback only returns the entity fact. Callers that providetitle+headingCountwithoutbodywill silently losedogfood.documentation.headingCount, so the block’s fact surface changes based on config shape instead of content.💡 Suggested fix
return { output: [ 'DocumentationArticleBlock', `title: ${title}`, `headings: ${headingCount}`, 'Intents: select heading; open reference', ].join('\n'), - facts: [{ kind: 'entity', key: 'dogfood.block', value: 'DocumentationArticleBlock' }], + facts: [ + { kind: 'entity', key: 'dogfood.block', value: 'DocumentationArticleBlock' }, + { kind: 'state', key: 'dogfood.documentation.headingCount', value: String(headingCount) }, + ], }; }🤖 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 1144 - 1152, The current return value for DocumentationArticleBlock drops the headingCount fact when body is absent; update the return so that if headingCount (from DocumentationArticleBlockConfig) is present you include a facts entry with key 'dogfood.documentation.headingCount' and its numeric/value (in addition to the existing 'dogfood.block' entity) so callers providing title + headingCount still receive that fact; locate the return in the function that builds the block output (where output and facts are constructed) and append or conditionally push the headingCount fact into the facts array when headingCount is defined.
🧹 Nitpick comments (1)
tests/cycles/DX-031/dogfood-blocks-section.test.ts (1)
320-321: 💤 Low valueRemove redundant assertion.
Lines 320 and 321 assert the identical condition—the template literal on line 320 produces the same string as the direct variable reference on line 321.
♻️ Proposed fix
- expect(text).toContain(`${block.metadata.blockName}`); expect(text).toContain(block.metadata.blockName);🤖 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 `@tests/cycles/DX-031/dogfood-blocks-section.test.ts` around lines 320 - 321, Remove the redundant assertion that checks the same string twice: keep a single expect on the rendered text containing the block name. Specifically, delete one of the two lines asserting expect(text).toContain(`${block.metadata.blockName}`) / expect(text).toContain(block.metadata.blockName) and retain the simpler expect(text).toContain(block.metadata.blockName) in the dogfood-blocks-section.test.ts test so the test still validates presence of block.metadata.blockName once.
🤖 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/app.ts`:
- Around line 3147-3159: The nav marker uses model.selectedGuideId for
activeItemId even in pipe/accessible modes, but keyboard nav updates
model.guideState.focusIndex; update the navigationListBlock.render call so that
when ctx.mode is 'pipe' or 'accessible' it derives activeItemId from
model.guideState.items[model.guideState.focusIndex].value (fallback to
model.selectedGuideId or undefined if out of range), otherwise keep using
model.selectedGuideId; change the activeItemId assignment in the config passed
to navigationListBlock.render to implement this conditional selection so the
marker follows focus during guide-next/guide-prev.
In `@examples/docs/dogfood-blocks.ts`:
- Around line 1165-1175: The sectioned-output branch that builds the
pipe/accessible string (inside the if sections.length > 0) only concatenates
section.title plus row.label and row.valueLabel, dropping row.description;
update the construction for sections.flatMap(...) (and mirror the same change in
the other section render path around lines referencing sections and rows) to
include row.description when present (e.g., append it after the valueLabel or on
a new line) so both the pipe/accessible and non-pipe section renderings expand
rows to include row.description as well as label and valueLabel.
In `@examples/docs/node-locale.ts`:
- Around line 36-44: The readText implementation in NodeDogfoodLocaleStorage
currently rethrows any fs error other than ENOENT; change readText to treat all
read errors as best-effort fallbacks by returning undefined (not throwing) for
non-ENOENT errors so locale resolution can continue; likewise update
savePreferredLocale to wrap the call to storage.writeText(...) in a try/catch
and swallow/log errors (do not allow them to propagate) so an unwritable
preference file does not abort locale switching. Ensure you modify the
NodeDogfoodLocaleStorage.readText function and the savePreferredLocale path that
calls storage.writeText to implement these best-effort behaviors.
In `@packages/bijou-tui/src/surface-layout.ts`:
- Around line 35-36: The width computed in proseSurface is not guarded against
non-finite values (NaN/Infinity) before calling
wrapToWidth/parseAnsiToSurface/createSurface; change the width calculation in
proseSurface to first coerce and validate options.width (e.g. use
Number.isFinite) and fall back to a safe default before Math.floor/Math.max so
wrapToWidth always receives a finite positive integer; ensure the check
references proseSurface and wrapToWidth (and indirectly
parseAnsiToSurface/createSurface) so the sanitized finite width is propagated to
those functions.
---
Outside diff comments:
In `@examples/docs/dogfood-blocks.ts`:
- Around line 1144-1152: The current return value for DocumentationArticleBlock
drops the headingCount fact when body is absent; update the return so that if
headingCount (from DocumentationArticleBlockConfig) is present you include a
facts entry with key 'dogfood.documentation.headingCount' and its numeric/value
(in addition to the existing 'dogfood.block' entity) so callers providing title
+ headingCount still receive that fact; locate the return in the function that
builds the block output (where output and facts are constructed) and append or
conditionally push the headingCount fact into the facts array when headingCount
is defined.
---
Nitpick comments:
In `@tests/cycles/DX-031/dogfood-blocks-section.test.ts`:
- Around line 320-321: Remove the redundant assertion that checks the same
string twice: keep a single expect on the rendered text containing the block
name. Specifically, delete one of the two lines asserting
expect(text).toContain(`${block.metadata.blockName}`) /
expect(text).toContain(block.metadata.blockName) and retain the simpler
expect(text).toContain(block.metadata.blockName) in the
dogfood-blocks-section.test.ts test so the test still validates presence of
block.metadata.blockName once.
🪄 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: 72bb3947-7cf4-4650-a89e-acc35a9c5758
⛔ Files ignored due to path filters (1)
examples/docs/i18n/source/dogfood-strings.csvis excluded by!**/*.csv
📒 Files selected for processing (24)
docs/BEARING.mddocs/CHANGELOG.mddocs/design/DF-070-dogfood-block-product-polish.mddocs/legends/DF-dogfood-field-guide.mdexamples/_shared/example-surfaces.tsexamples/docs/app.tsexamples/docs/dogfood-blocks.tsexamples/docs/i18n-debt.tsexamples/docs/i18n/catalogs/de/bijou.dogfood.jsonexamples/docs/i18n/catalogs/en/bijou.dogfood.jsonexamples/docs/i18n/catalogs/es/bijou.dogfood.jsonexamples/docs/i18n/catalogs/fr/bijou.dogfood.jsonexamples/docs/locale.tsexamples/docs/node-locale.tsexamples/docs/storybook-app.tspackages/bijou-tui/GUIDE.mdpackages/bijou-tui/README.mdpackages/bijou-tui/src/index.tspackages/bijou-tui/src/surface-layout.test.tspackages/bijou-tui/src/surface-layout.tstests/cycles/DF-068/dogfood-block-preview-regressions.test.tstests/cycles/DF-070/dogfood-block-product-polish.test.tstests/cycles/DX-031/dogfood-blocks-section.test.tstests/cycles/LX-011/dogfood-locale-ratchet.test.ts
Owner
Author
|
@codex Addressed the latest CodeRabbit/Codex feedback in follow-up commits:
Validation: focused regression suites, docs inventory, git diff --check, typecheck:test, lint, and pre-push full test suite + interactive examples all passed. |
Summary
Testing / Checks
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes DF-070 as a 10-slice DOGFOOD product-polish stack. This branch moves visible DOGFOOD documentation surfaces closer to the block-authored product direction without adding provider runtime, subscriptions, command dispatch, or broad AppShell rendering scope.
What changed:
proseSurface()in@flyingrobots/bijou-tuiand routes DOGFOOD prose through it.Non-goals
Validation
git diff --checknpm run docs:inventorynpm run dogfood:i18n:checknpm run dogfood:i18n:debtnpm test -- --run packages/bijou-tui/src/surface-layout.test.ts scripts/docs-preview.test.ts tests/cycles/DF-068/dogfood-block-preview-regressions.test.ts tests/cycles/DF-070/dogfood-block-product-polish.test.ts tests/cycles/DX-031/dogfood-blocks-section.test.ts tests/cycles/LX-011/dogfood-locale-ratchet.test.ts tests/cycles/LX-012/dogfood-i18n-debt-inventory.test.tsnpm run typecheck:testnpm run lintnpm test(312 files, 3513 tests)npm run verify:interactive-examplesnpm run typecheck:test,npm test, andnpm run verify:interactive-examples.Summary by CodeRabbit
New Features
Documentation
Tests