[nortonandreev]

#1157 — AI-3216

#1157 is already closed by #1158, so this line has no closing keyword.

What & why

Continuation of #1158, which merged before the reader spacing was finished. Lists sit on the section edge. Only Overview, which has no row padding, keeps a small top inset. The Reviewers, Submitted, and Threads row keeps a little space under the pills, and the loading notice sizes to its text.

Where to look

The review row's bottom margin is the gap under the pills. The scroll area has no top inset except the overview block.

Visuals

[nortonandreev]

Changelog

• Lists in the reader sit on the section edge. Only Overview keeps a small top inset.
• The Reviewers, Submitted, and Threads row has a little space under the pills.
• The loading notice sizes to its text.

[qodo-code-review]

PR Summary by Qodo

Show Hidden Resolved Thread Count in Threads Toggle

✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

AI Description

• Display the hidden resolved-thread count in the Threads toggle across filter changes.
• Separate review controls from thread content with a divider and padding.
• Cover excluded-thread reporting and toggle persistence with presentation tests.

Diagram

graph TD
  A["Unresolved Page"] -->|excluded count| B["Section Cache"] -->|projects label| C["View Model"] -->|binds text| D["Threads Toggle"]

Loading

High-Level Assessment

The current approach is appropriate: reading from the cached unresolved section preserves the only authoritative excluded-thread count when the active view switches to all threads. Maintaining a separate count field would duplicate section state without improving behavior.

Files changed (4) +29 / -6

Enhancement (2) +12 / -5

PullRequestContextViewModel.Projections.csProject the hidden resolved-thread count into the toggle label +5/-1

Project the hidden resolved-thread count into the toggle label

• Adds ShowResolvedLabel, sourced specifically from the cached unresolved thread section’s exact excluded count. Registers the projection for property-change notifications so the bound label refreshes with reader state.

src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs

PullRequestReader.axamlBind the resolved toggle label and separate the review bar +7/-4

Bind the resolved toggle label and separate the review bar

• Binds the Threads toggle caption to ShowResolvedLabel. Wraps the review controls in a padded bottom border to visually separate them from thread content.

src/Capacitor.App/Views/PullRequestReader.axaml

Tests (2) +17 / -1

FakePullRequestSource.csSimulate resolved threads excluded by the unresolved filter +3/-1

Simulate resolved threads excluded by the unresolved filter

• Adds a configurable hidden-resolved count and reports it through ExcludedByFilter only for unresolved thread requests.

test/Capacitor.App.Tests.Unit/FakePullRequestSource.cs

PullRequestPresentationTests.csVerify the hidden count persists when resolved threads are shown +14/-0

Verify the hidden count persists when resolved threads are shown

• Adds a presentation test confirming the toggle displays the excluded resolved-thread count before and after enabling the all-threads view.

test/Capacitor.App.Tests.Unit/PullRequestPresentationTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. A fake field comment adds no context ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

ResolvedHidden is preceded by an XML comment that only restates that it counts resolved threads
omitted by the unresolved view. The assignment in PageAsync already makes that condition explicit,
so a later change gets no invariant or usage constraint from the comment.

Code

test/Capacitor.App.Tests.Unit/FakePullRequestSource.cs[R17-18]

+    /// Resolved threads the unresolved view leaves out.
+    public int ResolvedHidden;

Relevance

●●● Strong

The team accepts removing comments that merely restate nearby test behavior instead of documenting a
non-obvious invariant.

PR-#1144
PR-#831

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Compliance rule 2762993 requires comments to document non-obvious, behavior-critical constraints
rather than restating clear code. The added comment merely paraphrases ResolvedHidden and the
explicit unresolved-thread condition in PageAsync.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.App.Tests.Unit/FakePullRequestSource.cs[17-18]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The XML comment on `ResolvedHidden` restates behavior already expressed by the field name and its conditional use in `PageAsync`, without documenting a non-obvious constraint.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/FakePullRequestSource.cs[17-18]

## Recommended Fix
Remove the redundant XML comment while retaining the `ResolvedHidden` field.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Screen readers miss hidden count 🐞 Bug ≡ Correctness

Description

The bound TextBlock now displays ShowResolvedLabel, while ShowResolvedToggle still overrides
its automation name with the fixed text “Show resolved threads.” When a hidden count is available,
keyboard and screen-reader focus on the toggle therefore exposes none of the new numerical
information.

Code

src/Capacitor.App/Views/PullRequestReader.axaml[73]

+                    <TextBlock Text="{Binding ShowResolvedLabel}" FontSize="12.5" Foreground="{StaticResource KcapMutedBrush}" VerticalAlignment="Center" />

Relevance

●●● Strong

Accessibility findings exposing missing keyboard or screen-reader behavior are accepted when
controls hide actionable information.

PR-#985
PR-#1030

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The dynamic value is bound only to the adjacent text element, whereas the focusable toggle has an
explicit fixed automation name that does not reference the new property.

src/Capacitor.App/Views/PullRequestReader.axaml[70-75]
src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs[56-59]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The visible toggle label includes the hidden resolved-thread count, but the toggle's explicit accessibility name remains static and omits that count.

## Fix Focus Areas
- src/Capacitor.App/Views/PullRequestReader.axaml[73-75]

## Recommended Fix
Bind `AutomationProperties.Name` to an accessible label derived from `ShowResolvedLabel`, preserving the word “threads” if needed, so the visible and announced labels both include the count.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Toggle reports too few threads ✗ Dismissed 🐞 Bug ≡ Correctness

Description

ShowResolvedLabel treats the unresolved section's Excluded value as a section-wide count, but
ThreadsAsync resets that value for every request and marks it exact after scanning only the
current GraphQL pages. Pull requests spanning multiple connection pages can therefore show a partial
count initially, then replace it with a continuation-local count when the user loads more threads.

Code

src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs[R58-59]

+    public string ShowResolvedLabel => CanDisplayReader && _sections.GetValueOrDefault("threads:unresolved")?.Excluded is { Kind: "exact", Value: > 0 } hidden
+        ? $"Show resolved ({hidden.Value})" : "Show resolved";

Relevance

●●● Strong

Recent accepted precedents prioritize fixing stale cached state and pagination or continuation race
conditions.

PR-#1158
PR-#1087

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The label directly displays the unresolved cache's exclusion value, while every page application
replaces that value. The CLI provider initializes the counter to zero for each call, stops once it
finds any visible item, and returns the locally observed value as exact; the repository design
defines this field as the count of published rows excluded by the filter rather than a page-local
count.

src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs[56-59]
src/Capacitor.App/ViewModels/PullRequestContextViewModel.Reads.cs[127-145]
src/Capacitor.Cli.Core/PullRequests/Readers/GitHubCli/GitHubCliReaderProvider.cs[148-176]
docs/superpowers/specs/2026-09-07-desktop-pr-context-design.md[480-487]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new toggle presents `ExcludedByFilter` as the total number of hidden resolved threads, but the GitHub CLI reader calculates it independently for each partial request and the view model overwrites the previous value during pagination.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs[58-59]
- src/Capacitor.App/ViewModels/PullRequestContextViewModel.Reads.cs[143-143]
- src/Capacitor.Cli.Core/PullRequests/Readers/GitHubCli/GitHubCliReaderProvider.cs[150-174]

## Recommended Fix
Make `ExcludedByFilter` snapshot-wide and stable across all continuation pages before labeling it `exact`, retaining the same count when later pages load. If the provider has not traversed enough of the connection to know the complete count, return `unknown` instead of displaying a partial value as exact.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

Review mode: ⚖️ Balanced: This is a localized UI/view-model behavior change with test coverage, but it alters state-dependent labeling and Avalonia layout binding, warranting a careful single-pass review.

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-code-review]

3. Screen readers miss hidden count 🐞 Bug ≡ Correctness

The bound TextBlock now displays ShowResolvedLabel, while ShowResolvedToggle still overrides
its automation name with the fixed text “Show resolved threads.” When a hidden count is available,
keyboard and screen-reader focus on the toggle therefore exposes none of the new numerical
information.

Agent Prompt

## Issue description
The visible toggle label includes the hidden resolved-thread count, but the toggle's explicit accessibility name remains static and omits that count.

## Fix Focus Areas
- src/Capacitor.App/Views/PullRequestReader.axaml[73-75]

## Recommended Fix
Bind `AutomationProperties.Name` to an accessible label derived from `ShowResolvedLabel`, preserving the word “threads” if needed, so the visible and announced labels both include the count.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[nortonandreev]

The count is the text beside the switch. The switch's accessible name stays "Show resolved threads", so focusing the switch does not announce the number. I can bind that name to the same label if the switch should say it.