Add markdown preview type and per result preview visibility control

[TrueCrimeDev]

What

The preview pane can now render formatted markdown with syntax-highlighted code blocks. Plugins opt in by setting Result.Preview.ContentType to Markdown and providing a markdown description.

A new Result.PreviewVisibility lets plugins control whether the preview pane shows:

• Optional (default) - respects the user's Always Preview setting and Preview toggle (Renamed to "Always show optional previews" & "Toggle Optional Previews")
• Always - forces the pane open so users see the preview without manual toggling
• Never - hides the pane for results with nothing useful to preview

And PreviewContentType controls how the preview is rendered:

• ImageWithText (default) - the classic preview: image above, plain text below
• Markdown - formatted markdown with headings, links, and syntax-highlighted code blocks

Existing plugins are unaffected as all defaults match current behavior.

Why

Plugins that produce rich text (AI assistants, documentation/snippet/note search, dictionary-style lookups) currently have to cram everything into plain SubTitle/Description text. This lets them opt into a proper rendered preview per result without affecting any existing plugin: every current result keeps the default text behavior.

The visibility controls exist so plugins can make sure users see the preview when it matters, without relying on them to manually toggle it or change settings. Results that exist primarily for their rendered content (like markdown) always show their preview; results with nothing useful to preview can hide it.

Details

Markdown rendering — Built on MdXaml (already a dependency) with code blocks highlighted through AvalonEdit (the one new package). A CodeHighlightTheme setting under Settings > Theme lets users pick a syntax theme or use Auto to match the app colour scheme. Bundled themes include VS Code Dark+, VS Code Light, One Dark, and Catppuccin Macchiato.

JSON-RPC — Both contentType and previewVisibility are added as keys on the result object (with contentType nested in preview):

{ "preview": { "contentType": "markdown", "description": "**hello**" }, "previewVisibility": "always" }

Themed horizontal scrollbars — Code blocks need horizontal scrolling, so horizontal scrollbar styles were added to Base.xaml and every built-in theme. Minimum thickness is 5px since horizontal bars are harder to click and have no scrollwheel alternative.

New dependency — AvalonEdit 6.3.0.90.

Tests

Full suite passes (256/256), including new coverage: pane gating rules (MainViewModelPreviewTest), markdown control behavior (PreviewMarkdownScrollViewerTest, PreviewMarkdownStyleTest), and JSON-RPC contentType deserialization (JsonRPCPluginTest).

Demo

Slightly outdated but still a good demonstration
📹 markdown-preview-demo.mp4

---

Summary by cubic

Adds a themed markdown preview pane with per-result visibility and a selectable code highlight theme. Refines preview behavior, accessibility, theming, and error handling; existing results keep the default imageWithText preview.

Summary of changes

• Changed
  • Result instances now create a fresh Preview object; Result.Clone() deep-copies Preview to prevent shared state (fixes BUG: Mutable PreviewInfo.Default shallow copying #4561).
  • Preview gating now uses a single _manualPreviewOverride: Never → hide, Always → show, else _manualPreviewOverride ?? AlwaysPreview. Toggle flips; Reset clears and re‑evaluates. Pane hides on window close; manual close persists even if Always Preview is on; forced previews restore on window reopen.
  • Default preview content type renamed to imageWithText; PreviewVisibility.Default renamed to Optional.
  • UI: wider preview column; preview min/max height bound to result list height; keep link underlines; only code‑block navigation keys are captured; window key handler is skipped while a code block is focused.
  • Links: validate http/https and open on click.
  • Scrollbars: themed horizontal scrollbars via dynamic keys HorizontalScrollBarStyle and HorizontalThumbStyle; explicit code‑block scrollbar templates prevent last‑line overlap; styles added to all built‑in themes with subtle thumbnails.
  • Error handling: UpdatePreviewAsync calls in MainViewModel PropertyChanged handlers are wrapped in try/catch; async preview methods are awaited (fire‑and‑forget only at WPF boundaries). Reset closes any external preview before opening an internal one.
  • Labels stay as “Always Preview” and “Toggle Preview”; tooltips note some results may override visibility. Localization and README updated.
• Added
  • Result.Preview.ContentType (imageWithText, markdown) and Result.PreviewVisibility (optional, never, always) with JSON‑RPC string values.
  • PreviewMarkdownScrollViewer using @MdXaml + AvalonEdit, per‑editor highlighting colorizer/theming, language aliases, themed code‑block scrollbars, and IsCodeBlockFocused.
  • Settings.CodeHighlightTheme (default "Auto") with a Theme page dropdown; Auto tracks app light/dark. Bundled themes: VS Code Light, VS Code Dark+, Catppuccin Macchiato, One Dark. Applies on startup and on theme change.
  • PreviewMarkdownStyle and base/theme resources for consistent markdown styling and horizontal scrollbars. Dependency: AvalonEdit 6.3.0.90.
• Removed
  • Old hidden preview content type (use PreviewVisibility.Never).
  • HidePreviewPane/ForcePreviewPane wrappers (inline PreviewVisibility access).
  • IsSafeScheme helper (inlined; links restricted to http/https).
• Memory
  • Small increase from AvalonEdit, per‑editor colorizers, and style/theme resources. Controls are created on demand.
• Security
  • Low risk. Markdown renders to a FlowDocument (no scripts/autonavigation). Links are http/https‑only and open on click. Code blocks are read‑only.
• Unit tests
  • Added/updated: MainViewModelPreviewTest (override logic, gating, restore on reopen, reset with stale external preview), PreviewMarkdownScrollViewerTest, PreviewMarkdownStyleTest, CodeHighlightThemeTest (Auto + unrecognized setting), JsonRPCPluginTest (imageWithText defaults and previewVisibility round‑trips with optional/never/always).

Release Note
You can see rich markdown previews with clickable links and highlighted code, choose a code theme, and control which results show a preview.

^{Written for commit 9a7f95d. Summary will update on new commits.}

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds per-result preview rendering modes (text/markdown/hidden) and introduces a markdown-capable internal preview pane with code highlighting, including updated preview-pane behavior and new tests.

Changes:

• Add PreviewContentType to plugin Result.PreviewInfo and surface flags in ResultViewModel for markdown/hidden handling.
• Implement PreviewMarkdownScrollViewer + theme resources to render markdown (with AvalonEdit fenced code blocks) in the preview pane.
• Update MainViewModel preview-open/close logic to support per-result suppression and markdown auto-open/close; add NUnit coverage.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
Flow.Launcher/packages.lock.json	Locks AvalonEdit as a direct dependency and updates plugin dependency range.
Flow.Launcher/Flow.Launcher.csproj	Adds AvalonEdit package reference required for markdown code blocks.
Flow.Launcher.Plugin/Result.cs	Adds PreviewContentType enum and JSON serialization support on PreviewInfo.
Flow.Launcher/ViewModel/ResultViewModel.cs	Exposes IsMarkdownPreview / HidePreviewPane for UI + preview logic.
Flow.Launcher/ViewModel/MainViewModel.cs	Implements suppression/auto-open behavior for preview based on selected result.
Flow.Launcher/MainWindow.xaml	Wires in PreviewMarkdownScrollViewer and toggles layout/visibility for markdown vs default preview.
Flow.Launcher/Themes/Base.xaml	Adds PreviewMarkdownStyle including hyperlink, blockquote, and code block styling.
Flow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.cs	New control to render markdown and apply compatibility fixes + syntax recoloring.
Flow.Launcher/Resources/Controls/CodeHighlightTheme.cs	Defines code highlight palettes and name mapping for AvalonEdit token categories.
Flow.Launcher.Test/PreviewMarkdownStyleTest.cs	Validates key style setters in Base.xaml for markdown rendering.
Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs	Validates hyperlink accent color + document page width sizing behavior.
Flow.Launcher.Test/Plugins/JsonRPCPluginTest.cs	Adds JSON-RPC deserialization test for contentType: "markdown".
Flow.Launcher.Test/MainViewModelPreviewTest.cs	Adds behavioral tests for hidden/markdown preview suppression + restoration logic.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

All reported issues were addressed across 13 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR adds markdown preview rendering, preview visibility controls, code highlighting theme selection, UI wiring, and supporting tests across the preview pipeline.

Changes

Markdown Preview Feature

Layer / File(s)	Summary
Preview contract and view model Flow.Launcher.Plugin/Result.cs, Flow.Launcher/ViewModel/ResultViewModel.cs	Result adds PreviewVisibility and PreviewInfo.ContentType, PreviewInfo.Default sets text content type, and ResultViewModel derives markdown and pane-visibility flags from preview metadata.
Markdown rendering and theming Flow.Launcher/Resources/Controls/CodeHighlightTheme.cs, Flow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.cs, Flow.Launcher/Themes/Base.xaml	Adds markdown document sizing, emphasis cleanup, embedded editor recoloring, AHK highlight aliases, theme presets and resolution, and the PreviewMarkdownStyle resource for markdown elements.
Theme settings and preview UI wiring Flow.Launcher.Infrastructure/UserSettings/Settings.cs, Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs, Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml, Flow.Launcher/MainWindow.xaml, Flow.Launcher/MainWindow.xaml.cs, Flow.Launcher/Flow.Launcher.csproj, Flow.Launcher/packages.lock.json, Flow.Launcher/Languages/en.xaml	Adds the code-highlight theme setting, exposes it in theme settings UI, applies it on load/theme changes, switches the main preview layout to the markdown viewer, and updates package/dependency metadata and localized labels.
Preview visibility control Flow.Launcher/ViewModel/MainViewModel.cs	Adds suppression state and result-driven preview pane behavior for hiding, restoring, forcing open, and auto-closing the preview.
Validation coverage Flow.Launcher.Test/MainViewModelPreviewTest.cs, Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs, Flow.Launcher.Test/PreviewMarkdownStyleTest.cs, Flow.Launcher.Test/Plugins/JsonRPCPluginTest.cs, Flow.Launcher.Test/CodeHighlightThemeTest.cs	Adds tests for preview visibility transitions, markdown rendering behavior, theme selection, XAML style values, and JSON-RPC markdown deserialization.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Flow-Launcher/Flow.Launcher#3867: Both PRs change how the preview pane is shown or hidden and wire preview-state logic into the main window flow.
• Flow-Launcher/Flow.Launcher#4167: Both PRs update preview-description plumbing and the main preview rendering path.
• Flow-Launcher/Flow.Launcher#3370: Both PRs modify preview-pane update flow in MainViewModel.

Suggested reviewers

• Jack251970
• onesounds
• VictoriousRaptor

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.78% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly matches the main change: markdown preview support plus per-result preview visibility control.
Description check	✅ Passed	The description is directly related to the changeset and describes the markdown preview and preview visibility feature.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs (1)

28-31: 💤 Low value

Consider a type check before casting Hyperlink.Foreground.

Line 29 casts hyperlink.Foreground to SolidColorBrush without verifying the type. While the test creates a style with SolidColorBrush, a pattern match or is check would make the test more robust against future style changes or theme variations.

♻️ Proposed fix using pattern matching

 var hyperlink = EnumerateInlines(viewer.Document.Blocks).OfType<Hyperlink>().Single();
-var foreground = (SolidColorBrush)hyperlink.Foreground;
-
-ClassicAssert.AreEqual(accentBrush.Color, foreground.Color);
+ClassicAssert.IsInstanceOf<SolidColorBrush>(hyperlink.Foreground);
+var foreground = (SolidColorBrush)hyperlink.Foreground;
+ClassicAssert.AreEqual(accentBrush.Color, foreground.Color);

🤖 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 `@Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs` around lines 28 - 31,
The test currently casts hyperlink.Foreground to SolidColorBrush without
verifying the type; update the assertion to first check the runtime type of
Hyperlink.Foreground (e.g., using pattern matching "is SolidColorBrush" or an
"as" check and a not-null/assert instance) before accessing Color, so replace
the direct cast of hyperlink.Foreground with a safe type check on
Hyperlink.Foreground and then compare the brush.Color to accentBrush.Color
(referencing the variables hyperlink, Hyperlink.Foreground, SolidColorBrush,
foreground and the assertion ClassicAssert.AreEqual).

Flow.Launcher.Test/MainViewModelPreviewTest.cs (1)

161-204: ⚡ Quick win

Consider guarding reflection calls against null to improve test diagnostics.

The helper methods use reflection to access private MainViewModel members by string name (backing fields, static fields, methods). If any of these members are renamed or removed during refactoring, the reflection calls will throw NullReferenceException at runtime instead of failing at compile time. Adding null checks with descriptive error messages would make test failures easier to diagnose.

🛡️ Example: Add null guard for GetField

 private static MainViewModel CreatePreviewViewModel(Settings settings, int resultAreaColumn)
 {
     var viewModel = (MainViewModel)RuntimeHelpers.GetUninitializedObject(typeof(MainViewModel));
-    typeof(MainViewModel)
+    var settingsField = typeof(MainViewModel)
         .GetField("<Settings>k__BackingField", BindingFlags.Instance | BindingFlags.NonPublic)
-        .SetValue(viewModel, settings);
+    if (settingsField == null)
+        throw new InvalidOperationException("MainViewModel.<Settings>k__BackingField not found; internal member may have been renamed.");
+    settingsField.SetValue(viewModel, settings);
     viewModel.ResultAreaColumn = resultAreaColumn;
     return viewModel;
 }

Apply similar guards to GetMethod and other GetField calls.

🤖 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 `@Flow.Launcher.Test/MainViewModelPreviewTest.cs` around lines 161 - 204, Guard
every reflection lookup and invocation in CreatePreviewViewModel,
ResultAreaColumnPreviewShown, ResultAreaColumnPreviewHidden,
InvokeUpdatePreviewAsync, and SetExternalPreviewVisible by checking the return
of Type.GetField/GetMethod for null and throwing a descriptive exception (e.g.,
InvalidOperationException) that names the missing member string (like
"<Settings>k__BackingField", "ResultAreaColumnPreviewShown",
"ResultAreaColumnPreviewHidden", "UpdatePreviewAsync",
"<ExternalPreviewVisible>k__BackingField") and the target type MainViewModel;
also validate MethodInfo.Invoke returns non-null where you expect a Task before
awaiting and provide clear messages for both missing members and unexpected
return types to make test failures easy to diagnose.

🤖 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 `@Flow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.cs`:
- Around line 217-228: The current code recreates a SolidHighlightingBrush each
loop and uses reference equality (Equals(color.Foreground, brush)), causing
unnecessary assignments and redraws; change the check to compare brush content
instead of object identity: inspect color.Foreground, cast it to
SolidHighlightingBrush (or check its exposed color property) and compare that
brush's color/value to the target before assigning; only set color.Foreground =
new SolidHighlightingBrush(target) and mark changed when the existing brush's
color differs (this prevents redundant NamedHighlightingColors rewrites and
avoids calling editor.TextArea.TextView.Redraw() when nothing changed).
- Line 19: ActiveTheme is hardcoded and not synced with the app theme, and
RetintEditor creates new SolidHighlightingBrush objects and compares by
reference which forces redundant Redraws; update ActiveTheme to read Flow's
current theme value (e.g., subscribe to the app/theme manager and set
PreviewMarkdownScrollViewer.ActiveTheme when the app theme changes) so the
control follows the selected app theme, and in RetintEditor stop allocating new
SolidHighlightingBrush for comparisons—reuse existing brushes or compare the
actual color values (e.g., compare color.Foreground.Color or brush.Color using
value equality) and only call editor.TextArea.TextView.Redraw() when the
color/value actually changed.

---

Nitpick comments:
In `@Flow.Launcher.Test/MainViewModelPreviewTest.cs`:
- Around line 161-204: Guard every reflection lookup and invocation in
CreatePreviewViewModel, ResultAreaColumnPreviewShown,
ResultAreaColumnPreviewHidden, InvokeUpdatePreviewAsync, and
SetExternalPreviewVisible by checking the return of Type.GetField/GetMethod for
null and throwing a descriptive exception (e.g., InvalidOperationException) that
names the missing member string (like "<Settings>k__BackingField",
"ResultAreaColumnPreviewShown", "ResultAreaColumnPreviewHidden",
"UpdatePreviewAsync", "<ExternalPreviewVisible>k__BackingField") and the target
type MainViewModel; also validate MethodInfo.Invoke returns non-null where you
expect a Task before awaiting and provide clear messages for both missing
members and unexpected return types to make test failures easy to diagnose.

In `@Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs`:
- Around line 28-31: The test currently casts hyperlink.Foreground to
SolidColorBrush without verifying the type; update the assertion to first check
the runtime type of Hyperlink.Foreground (e.g., using pattern matching "is
SolidColorBrush" or an "as" check and a not-null/assert instance) before
accessing Color, so replace the direct cast of hyperlink.Foreground with a safe
type check on Hyperlink.Foreground and then compare the brush.Color to
accentBrush.Color (referencing the variables hyperlink, Hyperlink.Foreground,
SolidColorBrush, foreground and the assertion ClassicAssert.AreEqual).

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9171e764-4894-436e-8051-480231edb557

📥 Commits

Reviewing files that changed from the base of the PR and between 5400502 and 3af13af.

📒 Files selected for processing (13)

• Flow.Launcher.Plugin/Result.cs
• Flow.Launcher.Test/MainViewModelPreviewTest.cs
• Flow.Launcher.Test/Plugins/JsonRPCPluginTest.cs
• Flow.Launcher.Test/PreviewMarkdownScrollViewerTest.cs
• Flow.Launcher.Test/PreviewMarkdownStyleTest.cs
• Flow.Launcher/Flow.Launcher.csproj
• Flow.Launcher/MainWindow.xaml
• Flow.Launcher/Resources/Controls/CodeHighlightTheme.cs
• Flow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.cs
• Flow.Launcher/Themes/Base.xaml
• Flow.Launcher/ViewModel/MainViewModel.cs
• Flow.Launcher/ViewModel/ResultViewModel.cs
• Flow.Launcher/packages.lock.json

[DavidGBrett]

Incase anyone else is reviewing this, heres the link to the plugin @TrueCrimeDev made using this feature:
https://github.com/TrueCrimeDev/Flow.Launcher.Plugin.Shorty

[cubic-dev-ai]

2 issues found across 14 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Flow.Launcher/ViewModel/MainViewModel.cs">

<violation number="1" location="Flow.Launcher/ViewModel/MainViewModel.cs:1152">
P2: Always Preview re-arms the preview pane after a manual close when passing through a hidden/Never result. SuppressPreviewAsync cannot distinguish a pane hidden by manual TogglePreview from one that was never opened, so the `else if (Settings.AlwaysPreview)` branch unconditionally sets `_restorePreviewAfterNeverResult = true` even after the user explicitly closed the preview. The next non-Never selection then reopens the pane via UpdatePreviewAsync, contradicting the PR's stated behavior that manual close should persist.</violation>

<violation number="2" location="Flow.Launcher/ViewModel/MainViewModel.cs:1220">
P2: ResetPreview's ForcePreviewPane branch can leave an existing external preview open because it bypasses HidePreview.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[coderabbitai]

Actionable comments posted: 3

🤖 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 `@Flow.Launcher.Plugin/Result.cs`:
- Line 380: The Result.Clone() implementation is still shallow-copying Preview,
and PreviewInfo.Default is being shared as a mutable singleton, so updates can
leak across results. Update Result.Clone() to deep-copy the PreviewInfo instead
of reusing the same instance, and replace the shared default preview object with
a fresh PreviewInfo created per Result (for example via
PreviewInfo.CreateDefault()) in the Result/PreviewInfo initialization path.
Refer to Result.Clone() and the PreviewInfo.Default/CreateDefault setup so each
result owns its own preview state.

In `@Flow.Launcher.Test/CodeHighlightThemeTest.cs`:
- Around line 34-39: The test in
`GivenUnknownOrEmptySetting_WhenApplied_ThenFallsBackToAutoBehaviour` only
covers the empty-string path, so add a separate assertion using a truly
unrecognized theme name such as a bogus value to exercise the unknown-setting
fallback in `PreviewMarkdownScrollViewer.ApplyCodeHighlightTheme`. Keep the
existing empty case if needed, but ensure the test explicitly verifies that an
invalid non-empty setting still resolves to the default theme via
`PreviewMarkdownScrollViewer.ActiveThemeName`.
- Around line 10-39: The tests in CodeHighlightThemeTest are mutating the static
theme state on PreviewMarkdownScrollViewer, which can leak between tests and
make the fixture order-dependent. Add a small setup/teardown in this test class
to save and restore the previous PreviewMarkdownScrollViewer.ActiveTheme (or
otherwise reset the theme after each test), so each
ApplyCodeHighlightTheme/ActiveThemeName assertion runs in isolation.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 90be7157-4534-4d06-b869-267e8d57511f

📥 Commits

Reviewing files that changed from the base of the PR and between 3af13af and 20c7c62.

📒 Files selected for processing (14)

• Flow.Launcher.Infrastructure/UserSettings/Settings.cs
• Flow.Launcher.Plugin/Result.cs
• Flow.Launcher.Test/CodeHighlightThemeTest.cs
• Flow.Launcher.Test/MainViewModelPreviewTest.cs
• Flow.Launcher.Test/Plugins/JsonRPCPluginTest.cs
• Flow.Launcher/Languages/en.xaml
• Flow.Launcher/MainWindow.xaml.cs
• Flow.Launcher/Resources/Controls/CodeHighlightTheme.cs
• Flow.Launcher/Resources/Controls/PreviewMarkdownScrollViewer.cs
• Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs
• Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml
• Flow.Launcher/Themes/Base.xaml
• Flow.Launcher/ViewModel/MainViewModel.cs
• Flow.Launcher/ViewModel/ResultViewModel.cs

✅ Files skipped from review due to trivial changes (2)

• Flow.Launcher/Languages/en.xaml
• Flow.Launcher.Infrastructure/UserSettings/Settings.cs

🚧 Files skipped from review as they are similar to previous changes (3)

• Flow.Launcher/Themes/Base.xaml
• Flow.Launcher.Test/Plugins/JsonRPCPluginTest.cs
• Flow.Launcher/ViewModel/MainViewModel.cs

[cubic-dev-ai]

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Flow.Launcher/MainWindow.xaml">

<violation number="1" location="Flow.Launcher/MainWindow.xaml:541">
P2: The preview-pane scrollbar uses a local implicit style that hardcodes the thumb color (#898989) and bypasses the theme system, contradicting the comment calling it a "themed scrollbar". Flow Launcher has extensive per-theme styling in Themes/Base.xaml; hardcoded colors here will not adapt to different themes and cannot be overridden by theme files.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Flow.Launcher/Themes/Base.xaml">

<violation number="1" location="Flow.Launcher/Themes/Base.xaml:617">
P2: Custom ScrollBar template omits DecreaseRepeatButton and IncreaseRepeatButton, breaking standard track/page-click behavior. Users can only drag the thumb, not click the track to page through content.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[TrueCrimeDev]

Thanks for the thorough review @DavidGBrett — all points addressed, with replies in each thread. Quick map:

• PreviewVisibility { Default, Never, Always } on Result replaces PreviewContentType.Hidden, decoupling pane visibility from content type (ea78b53)
• Code-highlight theme is now a Settings → Theme dropdown, with a new VS Code Light theme and an Auto (match color scheme) default (38c27fd)
• Preview pane reappears on reopen for forced previews, with regression tests (1504177)
• Preview + code-block scrollbars now use Flow's thin themed style; the code-block horizontal scrollbar sits in its own grid row so it can't cover the last line (7092ad2, 1ade524)
• Clarified the double preview-suppress check (renamed _restorePreviewAfterNeverResult + comments)

Two notes:

1. The scrollbar / last-line fixes are visual — I'd appreciate a quick check that they render as expected on your setup.
2. Behaviour change: content type no longer forces the pane open on its own; forced (e.g. markdown) previews now opt in via PreviewVisibility.Always. Migrated the Shorty plugin to match: Migrate to the PreviewVisibility API TrueCrimeDev/Flow.Launcher.Plugin.Shorty#4

[TrueCrimeDev]

@Flow-Launcher — this one's ready for another look whenever a maintainer has time.

All of @DavidGBrett's review feedback has been addressed (responses in each thread, summary above) and the branch is green. The only items worth a second pair of eyes are the two scrollbar visuals, which I've called out in-thread. Happy to make any further changes — thanks!

[DavidGBrett]

Thanks for sorting those out
I will look further into the key binding concerns you mentioned - the current behavior seems fine to me so far anyways.

Right now I've noticed an uncaught crash seemingly happening when the preview changes quickly so I'm focusing on trying to figure out what's causing that
To test it out you can use my own plugin which renders the query as the first line of the markdown so changing that changes the preview.

[DavidGBrett]

The crash is not easy to reliably replicate but it seems that this is actually caused by a bug in how MdXaml handles image loading
at MdXaml.ImageLoaderManager.OpenStreamAsync(Uri resourceUrl)
Some unhandled http request failure

My test plugin included a markdown image link
I only added that recently which explains why im only seeing it now.

The reason why this was crashing on my build was that for some reason in debug mode the top level exception handlers are disabled - unfortunately I would need to discuss with the team and fix that in a sperate PR so will have to leave this unaddressed here

It's less than ideal to leave it like this, but at least in release mode the crash shouldn't happen for normal users...

[DavidGBrett]

I've polished it up so I believe it should be good enough to merge now

A good few changes in the mean time, but I've updated the PR description so hopefully they should be clear.

If anyone else wants to review this remember to use the release build - dev build can crash as described above but that's not a concern for this PR I believe

[cubic-dev-ai]

1 issue found across 40 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Flow.Launcher.Test/MainViewModelPreviewTest.cs">

<violation number="1" location="Flow.Launcher.Test/MainViewModelPreviewTest.cs:331">
P2: Observation: The test helper creates a MainViewModel with RuntimeHelpers.GetUninitializedObject and then writes directly to the compiler-generated backing field "<Settings>k__BackingField". That bypasses the MainViewModel constructor and ties the test to a private backing-field name, which produces unrealistic objects and brittle tests that can break under refactoring.

Recommendation: Create the view model through a real constructor or a deliberate test seam instead of using GetUninitializedObject and touching backing fields. Options, in order of preference:
- Add an internal or protected constructor or a factory on MainViewModel that accepts Settings (and mark it InternalsVisibleTo the test assembly) and call that from tests.
- If a non-public constructor already exists, use Activator.CreateInstance(typeof(MainViewModel), nonPublic: true) so the constructor runs, then set observable properties via public API.
- As a last resort, if you must use reflection, set the public or non-public property (via GetProperty) rather than referencing the compiler-generated backing field name.

Making this change will ensure test instances reflect real runtime initialization and avoid brittle coupling to compiler-generated names.</violation>
</file>

<sub>Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Observation: The test helper creates a MainViewModel with RuntimeHelpers.GetUninitializedObject and then writes directly to the compiler-generated backing field "k__BackingField". That bypasses the MainViewModel constructor and ties the test to a private backing-field name, which produces unrealistic objects and brittle tests that can break under refactoring.

Recommendation: Create the view model through a real constructor or a deliberate test seam instead of using GetUninitializedObject and touching backing fields. Options, in order of preference:

• Add an internal or protected constructor or a factory on MainViewModel that accepts Settings (and mark it InternalsVisibleTo the test assembly) and call that from tests.
• If a non-public constructor already exists, use Activator.CreateInstance(typeof(MainViewModel), nonPublic: true) so the constructor runs, then set observable properties via public API.
• As a last resort, if you must use reflection, set the public or non-public property (via GetProperty) rather than referencing the compiler-generated backing field name.

Making this change will ensure test instances reflect real runtime initialization and avoid brittle coupling to compiler-generated names.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At Flow.Launcher.Test/MainViewModelPreviewTest.cs, line 331:

<comment>Observation: The test helper creates a MainViewModel with RuntimeHelpers.GetUninitializedObject and then writes directly to the compiler-generated backing field "<Settings>k__BackingField". That bypasses the MainViewModel constructor and ties the test to a private backing-field name, which produces unrealistic objects and brittle tests that can break under refactoring.

Recommendation: Create the view model through a real constructor or a deliberate test seam instead of using GetUninitializedObject and touching backing fields. Options, in order of preference:
- Add an internal or protected constructor or a factory on MainViewModel that accepts Settings (and mark it InternalsVisibleTo the test assembly) and call that from tests.
- If a non-public constructor already exists, use Activator.CreateInstance(typeof(MainViewModel), nonPublic: true) so the constructor runs, then set observable properties via public API.
- As a last resort, if you must use reflection, set the public or non-public property (via GetProperty) rather than referencing the compiler-generated backing field name.

Making this change will ensure test instances reflect real runtime initialization and avoid brittle coupling to compiler-generated names.</comment>

<file context>
@@ -0,0 +1,389 @@
+
+        private static MainViewModel CreatePreviewViewModel(Settings settings, int resultAreaColumn)
+        {
+            var viewModel = (MainViewModel)RuntimeHelpers.GetUninitializedObject(typeof(MainViewModel));
+            typeof(MainViewModel)
+                .GetField("<Settings>k__BackingField", BindingFlags.Instance | BindingFlags.NonPublic)
</file context>

[cubic-dev-ai]

All reported issues were addressed across 35 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>