[codex] strengthen bionify controls and settings#864
Merged
jeffscottward merged 6 commits intoRunMaestro:mainfrom Apr 19, 2026
Merged
[codex] strengthen bionify controls and settings#864jeffscottward merged 6 commits intoRunMaestro:mainfrom
jeffscottward merged 6 commits intoRunMaestro:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds persisted Bionify settings (intensity, algorithm), UI controls and validation, algorithm-driven per-word emphasis with CSS-variable styling, threads settings through markdown/preview/terminal renderers, updates components/styles, and expands unit and E2E tests including an AI chat flow. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as DisplayTab / FilePreview / Terminal UI
participant Store as SettingsStore
participant Renderer as MarkdownRenderer
participant Style as StyleInjector
User->>UI: change Intensity / edit Algorithm
UI->>Store: setBionifyIntensity(value), setBionifyAlgorithm(string)
Store-->>UI: persisted state updated
UI->>Renderer: render (reads store via hook)
Renderer->>Style: applyReadableTextTransforms(intensity, algorithm)
Style-->>Renderer: inject CSS vars / scoped styles
Renderer->>UI: render Bionify-marked nodes
UI-->>User: emphasized text (fontWeight / opacity)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
jeffscottward
force-pushed
the
codex/bionify-polish-intensity
branch
from
d4d71c0 to
bc428a8
Compare
Greptile SummaryThis PR strengthens the Bionify reading-mode feature by replacing the original hard-coded emphasis model ( All remaining findings are P2 (no blocking bugs): the algorithm free-text field silently falls back to defaults on invalid input without user feedback, the info-modal description of the fallback fraction says "fraction of words" where it should say "fraction of characters", and the module-level style-injection flag can cause test-ordering sensitivity. Confidence Score: 5/5Safe to merge; all findings are P2 style/UX improvements with no runtime correctness impact. No P0/P1 defects found. The algorithm parser falls back gracefully on malformed input, CSS variable cascade is correctly scoped, dependency arrays in both AutoRun useMemo blocks are complete, and the settings metadata defaults are consistent with the UI presets. src/renderer/components/Settings/tabs/DisplayTab.tsx (algorithm input validation and info-modal copy); src/renderer/utils/bionifyReadingMode.tsx (module-level injection flag in tests). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Store["settingsStore\n(bionifyReadingMode\nbionifyIntensity\nbionifyAlgorithm)"]
DisplayTab["DisplayTab\n(Reading Mode toggle\nIntensity slider\nAlgorithm input)"]
AutoRun["AutoRun.tsx"]
FilePreview["FilePreview.tsx"]
MarkdownConfig["markdownConfig.ts\n(BionifyRenderConfig)"]
BionifyUtil["bionifyReadingMode.tsx\nnormalizeBionifyConfig()\nparseBionifyAlgorithm()\nbuildBionifyCssVars()"]
BionifyText["BionifyText /\nBionifyTextBlock\n(CSS vars on span/div)"]
CSSVars["CSS Variables\n--bionify-emphasis-weight\n--bionify-rest-opacity"]
StyleTag["Injected style tag\n(singleton)"]
DisplayTab -->|read/write| Store
AutoRun -->|reads| Store
FilePreview -->|reads| Store
AutoRun -->|passes config| MarkdownConfig
FilePreview -->|passes config| MarkdownConfig
MarkdownConfig -->|BionifyRenderConfig| BionifyUtil
BionifyUtil --> BionifyText
BionifyText --> CSSVars
BionifyUtil --> StyleTag
Reviews (1): Last reviewed commit: "docs: refresh bionify verification scree..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/renderer/components/FilePreview.tsx (1)
936-996:⚠️ Potential issue | 🟠 MajorInclude Bionify settings in the memo dependencies.
bionifyIntensityandbionifyAlgorithmare read fromuseSettingsStoreand passed tocreateMarkdownComponents(lines 943–944), but the dependency array omits them. When the user changes Display settings, markdown previews continue rendering with stale Bionify values.Proposed fix
}, [ effectiveBionifyReadingMode, + bionifyIntensity, + bionifyAlgorithm, onFileClick, theme, cwd,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/FilePreview.tsx` around lines 936 - 996, The memo for markdownComponents omits bionifyIntensity and bionifyAlgorithm from its dependency array, causing stale Bionify settings; update the useMemo dependency list for markdownComponents to include bionifyIntensity and bionifyAlgorithm so that createMarkdownComponents is re-run when those settings change (refer to markdownComponents, createMarkdownComponents, bionifyIntensity, bionifyAlgorithm and the useMemo dependency array).
🧹 Nitpick comments (3)
src/renderer/utils/bionifyReadingMode.tsx (1)
125-128: Nit: drop the redundant* 1.
(intensity - 1) * 1is equivalent to(intensity - 1). Minor readability clean-up; the* 1may look like a forgotten slope/scale placeholder.♻️ Proposed diff
- return Number(clamp(baseOpacity - (intensity - 1) * 1, 0.2, 0.9).toFixed(2)); + return Number(clamp(baseOpacity - (intensity - 1), 0.2, 0.9).toFixed(2));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/utils/bionifyReadingMode.tsx` around lines 125 - 128, In function resolveBionifyRestOpacity remove the unnecessary multiplication by 1 in the expression `(intensity - 1) * 1` — replace it with `(intensity - 1)` to improve readability; confirm the logic remains the same and keep the clamp, toFixed, and theme-based baseOpacity handling unchanged.src/renderer/stores/settingsStore.ts (1)
1883-1983: ExposesetBionifyIntensity/setBionifyAlgorithmfromgetSettingsActions()for parity.The new setters are added to the store, but
getSettingsActions()— which other non-React code paths use to mutate settings — still only exposessetBionifyReadingMode(line 1908). Add the two new setters next to it so the API surface stays consistent with the rest of the bionify trio.♻️ Proposed diff
setBionifyReadingMode: state.setBionifyReadingMode, + setBionifyIntensity: state.setBionifyIntensity, + setBionifyAlgorithm: state.setBionifyAlgorithm, setShowHiddenFiles: state.setShowHiddenFiles,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/stores/settingsStore.ts` around lines 1883 - 1983, getSettingsActions() currently exposes setBionifyReadingMode but omits the new setters setBionifyIntensity and setBionifyAlgorithm; update the returned object in getSettingsActions to include state.setBionifyIntensity and state.setBionifyAlgorithm (place them next to setBionifyReadingMode for parity) so non-React code can mutate those bionify settings just like the others.e2e/bionify-reading-mode.spec.ts (1)
327-337: FragileBtoolbar locator and strict dimension equality.
{ has: window.locator('span', { hasText: 'B' }) }uses substring matching, so any span whose text contains the characterB(e.g., "Branch", "Build") will match, making.first()/.nth(1)rely on DOM ordering rather than identity. Prefer a more specific selector — e.g.,getByTitle('Bionify reading mode')/getByRole('button', { name: /^B$/i })/hasText: /^B$/— so the test fails loudly if the toggle is renamed or reordered.Additionally,
toBe(...)onboundingBox().height/widthis brittle against sub-pixel rendering differences. ConsidertoBeCloseTo(x, 0)to tolerate rounding while still asserting visual parity.♻️ Example adjustment
- const bionifyButtons = window - .locator('button') - .filter({ has: window.locator('span', { hasText: 'B' }) }); + const bionifyButtons = window + .locator('button') + .filter({ has: window.locator('span', { hasText: /^B$/ }) }); @@ - expect(filePreviewButtonMetrics[0]?.height).toBe(filePreviewButtonMetrics[1]?.height); - expect(filePreviewButtonMetrics[0]?.width).toBe(filePreviewButtonMetrics[1]?.width); + expect(filePreviewButtonMetrics[0]?.height).toBeCloseTo(filePreviewButtonMetrics[1]?.height ?? 0, 0); + expect(filePreviewButtonMetrics[0]?.width).toBeCloseTo(filePreviewButtonMetrics[1]?.width ?? 0, 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/bionify-reading-mode.spec.ts` around lines 327 - 337, The locator for the "B" toolbar button (bionifyButtons / filePreviewBeforeButton) is fragile because it uses substring matching via span hasText 'B' and .first(); replace that with a precise selector such as getByTitle('Bionify reading mode') or getByRole('button', { name: /^B$/i }) (or hasText: /^B$/) to target the intended toggle explicitly, and change the strict equality assertions on boundingBox().height/width to tolerant comparisons (e.g., use toBeCloseTo with precision 0) when comparing filePreviewButtonMetrics to account for sub-pixel rendering; keep the writeDurableScreenshot call and the getByTitle('Copy content to clipboard') reference as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/renderer/components/FilePreview.tsx`:
- Around line 936-996: The memo for markdownComponents omits bionifyIntensity
and bionifyAlgorithm from its dependency array, causing stale Bionify settings;
update the useMemo dependency list for markdownComponents to include
bionifyIntensity and bionifyAlgorithm so that createMarkdownComponents is re-run
when those settings change (refer to markdownComponents,
createMarkdownComponents, bionifyIntensity, bionifyAlgorithm and the useMemo
dependency array).
---
Nitpick comments:
In `@e2e/bionify-reading-mode.spec.ts`:
- Around line 327-337: The locator for the "B" toolbar button (bionifyButtons /
filePreviewBeforeButton) is fragile because it uses substring matching via span
hasText 'B' and .first(); replace that with a precise selector such as
getByTitle('Bionify reading mode') or getByRole('button', { name: /^B$/i }) (or
hasText: /^B$/) to target the intended toggle explicitly, and change the strict
equality assertions on boundingBox().height/width to tolerant comparisons (e.g.,
use toBeCloseTo with precision 0) when comparing filePreviewButtonMetrics to
account for sub-pixel rendering; keep the writeDurableScreenshot call and the
getByTitle('Copy content to clipboard') reference as-is.
In `@src/renderer/stores/settingsStore.ts`:
- Around line 1883-1983: getSettingsActions() currently exposes
setBionifyReadingMode but omits the new setters setBionifyIntensity and
setBionifyAlgorithm; update the returned object in getSettingsActions to include
state.setBionifyIntensity and state.setBionifyAlgorithm (place them next to
setBionifyReadingMode for parity) so non-React code can mutate those bionify
settings just like the others.
In `@src/renderer/utils/bionifyReadingMode.tsx`:
- Around line 125-128: In function resolveBionifyRestOpacity remove the
unnecessary multiplication by 1 in the expression `(intensity - 1) * 1` —
replace it with `(intensity - 1)` to improve readability; confirm the logic
remains the same and keep the clamp, toFixed, and theme-based baseOpacity
handling unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9685d534-ff0f-4e04-80bb-c78f12f83f0a
⛔ Files ignored due to path filters (8)
docs/screenshots/bionify-ai-chat-after.pngis excluded by!**/*.pngdocs/screenshots/bionify-ai-chat-before.pngis excluded by!**/*.pngdocs/screenshots/bionify-autorun-after.pngis excluded by!**/*.pngdocs/screenshots/bionify-autorun-before.pngis excluded by!**/*.pngdocs/screenshots/bionify-file-preview-after.pngis excluded by!**/*.pngdocs/screenshots/bionify-file-preview-before.pngis excluded by!**/*.pngdocs/screenshots/bionify-settings-default.pngis excluded by!**/*.pngdocs/screenshots/bionify-settings-info.pngis excluded by!**/*.png
📒 Files selected for processing (16)
e2e/bionify-reading-mode.spec.tssrc/__tests__/renderer/components/AutoRunDocumentSelector.test.tsxsrc/__tests__/renderer/components/FilePreview.test.tsxsrc/__tests__/renderer/components/Settings/tabs/DisplayTab.test.tsxsrc/__tests__/renderer/utils/bionifyReadingMode.test.tsxsrc/renderer/components/AutoRun.tsxsrc/renderer/components/AutoRunDocumentSelector.tsxsrc/renderer/components/FilePreview.tsxsrc/renderer/components/MarkdownRenderer.tsxsrc/renderer/components/Settings/tabs/DisplayTab.tsxsrc/renderer/components/TerminalOutput.tsxsrc/renderer/hooks/settings/useSettings.tssrc/renderer/stores/settingsStore.tssrc/renderer/utils/bionifyReadingMode.tsxsrc/renderer/utils/markdownConfig.tssrc/shared/settingsMetadata.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/bionify-reading-mode.spec.ts`:
- Around line 472-479: The current assertions call window.evaluate and use
document.querySelectorAll('.bionify-word-emphasis') which counts Bionify spans
across the whole document; change these to scope to the AI chat snippet
container by first locating that container (e.g., via a stable selector/test-id
for the AI chat snippet) and then querying inside it (for example call
window.evaluate(() =>
document.querySelector('<ai-chat-container-selector>').querySelectorAll('.bionify-word-emphasis').length)
or get a locator/elementHandle for the chat container and run container.evaluate
to count/style-check only its children). Apply this change to the two places
that use '.bionify-word-emphasis' (the existing window.evaluate count and the
later style checks) so the assertions only inspect the AI chat surface.
In `@src/renderer/stores/settingsStore.ts`:
- Around line 618-625: The setter setBionifyIntensity currently clamps the
incoming value but doesn't guard against NaN or non-numeric persisted values,
allowing NaN to be stored and propagated; update setBionifyIntensity to first
coerce/validate the incoming value with Number(value) (or isFinite) and if it's
not a finite number fall back to a safe default (e.g., 1.0) before applying
Math.max/Math.min, then call set({ bionifyIntensity: safeValue }) and
window.maestro.settings.set('bionifyIntensity', safeValue); apply the same
finite-value guard pattern to the other occurrence around lines 1521–1525 where
bionifyIntensity is set from persisted data.
In `@src/renderer/utils/bionifyReadingMode.tsx`:
- Around line 304-320: BionifyText currently renders spans with inline CSS vars
but doesn't ensure the shared bionify stylesheet is injected and the stylesheet
rules are scoped to .bionify-text-block only, so standalone BionifyText can be
visually inert; update BionifyText to ensure the shared stylesheet is injected
when it mounts (same mechanism that BionifyTextBlock uses) and change or add
rules in that stylesheet so they also target the BionifyText span (the element
rendered by BionifyText) in addition to .bionify-text-block; reference the
BionifyText component, buildBionifyCssVars (for inline vars) and
renderBionifyChildren to locate the rendering logic and reuse the stylesheet
injection helper used by BionifyTextBlock.
🪄 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: fe7c5560-5b91-4734-b5cb-9222a4406844
📒 Files selected for processing (7)
e2e/bionify-reading-mode.spec.tssrc/__tests__/renderer/components/Settings/tabs/DisplayTab.test.tsxsrc/__tests__/renderer/utils/bionifyReadingMode.test.tsxsrc/renderer/components/FilePreview.tsxsrc/renderer/components/Settings/tabs/DisplayTab.tsxsrc/renderer/stores/settingsStore.tssrc/renderer/utils/bionifyReadingMode.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/components/Settings/tabs/DisplayTab.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/utils/bionifyReadingMode.tsx`:
- Around line 334-372: BionifyTextBlock's explicit restOpacity is being
clobbered by the nested BionifyText which creates its own .bionify-text-block
and CSS vars; remove the inner wrapper and have BionifyTextBlock be the single
CSS-var scope by calling the low-level renderer (use renderBionifyChildren / the
same helper that BionifyText uses) instead of rendering <BionifyText>; keep
using normalizeBionifyConfig, buildBionifyCssVars and the blockClassName, apply
the restOpacity CSS var only on the outer div, and pass
enabled/intensity/algorithm into the render helper so children are rendered with
emphasis without adding another .bionify-text-block wrapper.
🪄 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: 1ad86a1a-c14c-4c99-a794-f2a608bfc589
📒 Files selected for processing (3)
e2e/bionify-reading-mode.spec.tssrc/renderer/stores/settingsStore.tssrc/renderer/utils/bionifyReadingMode.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/stores/settingsStore.ts
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
This PR tightens the Bionify desktop experience across the three surfaces from the screenshots and adds stronger configuration controls.
Btoggle geometry in File Preview and Auto Run so it matches adjacent toolbar buttonsRoot Cause
The original implementation had two separate problems:
Bcontrols rendered taller and visually narrower than neighboring icon buttons.font-weight: 600, high rest opacity, fixed 1-4 character emphasis) that produced almost no perceptible change in real UI surfaces.Impact
Screenshot Artifacts
docs/screenshots/bionify-settings-default.pngdocs/screenshots/bionify-settings-info.pngdocs/screenshots/bionify-file-preview-before.pngdocs/screenshots/bionify-file-preview-after.pngdocs/screenshots/bionify-autorun-before.pngdocs/screenshots/bionify-autorun-after.pngdocs/screenshots/bionify-ai-chat-before.pngdocs/screenshots/bionify-ai-chat-after.pngValidation
npx vitest run src/__tests__/renderer/utils/bionifyReadingMode.test.tsx src/__tests__/renderer/components/FilePreview.test.tsx src/__tests__/renderer/components/AutoRunDocumentSelector.test.tsx src/__tests__/renderer/components/Settings/tabs/DisplayTab.test.tsxMAESTRO_WRITE_DURABLE_SCREENSHOTS=true npx playwright test e2e/bionify-reading-mode.spec.tsnpm run lintnpm run lint:eslint(passes with one unrelated pre-existing warning insrc/main/web-server/web-server-factory.ts)npm run validate:pushSummary by CodeRabbit
New Features
Tests
Style