Skip to content

Admit a partial answer once the memory pass has run - #9

Merged
donislawdev merged 1 commit into
mainfrom
fix/partial-notice-after-extra-read
Sep 23, 2026
Merged

donislawdev merged 1 commit into
mainfrom
fix/partial-notice-after-extra-read

Conversation

@donislawdev

@donislawdev donislawdev commented Sep 23, 2026

Copy link
Copy Markdown
Owner

What was wrong

A window without administrator rights answered status:running memory:>100MB with "Nothing in this list matches what you asked for" and said nothing else. On the machine this was found on, Windows had refused the memory of 104 service processes (memory:? counts them), so "no service holds more than 100 MB" read as the answer while most running services were never judged.

The sentence that admits a partial answer was held back for every query needing a second phase family (memory or signatures), including after that family had been read. The query engine counts a refused field and a field nobody read as one number, and the hold-back existed so that "nobody looked" would not be reported as a refusal. Once the pass has run, what is left unread is a refusal, and the hold-back had no reason to stay.

A shown Memory column sets the same need, so it silenced the sentence for questions about any other field too.

What changed

  • Sentences.Admissions holds the sentence back only while a family the query needs has not been read: (needs & ~have) == ExtraRead.None instead of needs == ExtraRead.None.
  • The command line already printed this warning on any count, so both interfaces now agree on the same answer.
  • CHANGELOG.md: one entry under Fixed.

Tests

Four new tests in AdmissionTests:

  • a refusal the memory pass brought back is admitted (fails on the previous code),
  • a shown memory column does not silence a refusal about another field (fails on the previous code),
  • a pass that read every process admits nothing,
  • while one needed family is still unread the refusal sentence stays back - and the test proves memory was read, so it sits on the boundary it names.

Checked on the live window without administrator rights: the same query now says "104 entries were judged on a field that could not be read, so this list may be missing entries, or holding ones that do not belong."

Four window tests read the real process token and fail on any session without administrator rights. They fail identically on the code before this change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Partial-results notices now accurately report entries that could not be evaluated due to unreadable memory, including when the Memory column is displayed alongside a query for another field.
    • Notices appear after required data has been read, while remaining suppressed if necessary data is still unavailable.
  • Documentation
    • Added a changelog entry describing the unreadable-memory notice.

A window without administrator rights answered "status:running memory:>100MB"
with an empty list and nothing else, while Windows had refused the memory of
most service processes. The sentence that says an answer rests on fields that
could not be read was held back for every query needing a second phase family,
even after that family had been read - when what is left unread is a refusal.

The sentence is now held back only while a family the query needs is still
unread. A shown Memory column, which asks for the same family, no longer
silences it for questions about other fields either. The command line already
said this on any count, so both interfaces now agree on the same answer.

Four tests pin the change: the refusal after the pass, a pass that read
everything, a family still unread, and the shown column. The first and the
last fail on the previous code.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7dc244ff-b6d8-4206-8a17-ef2b20fe5ab0

📥 Commits

Reviewing files that changed from the base of the PR and between eba59a3 and 2e75484.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/Bws.Gui/ViewModels/Sentences.cs
  • tests/Bws.Gui.Tests/AdmissionTests.cs

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (5)
  • GitHub Check: Analyse csharp
  • GitHub Check: Analyse actions
  • GitHub Check: build and the tests that do not need this machine
  • GitHub Check: Dependency review
  • GitHub Check: Semgrep
🧰 Additional context used
📓 Path-based instructions (10)
Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).

⚙️ CodeRabbit configuration file

Files:

  • src/Bws.Gui/ViewModels/Sentences.cs
  • tests/Bws.Gui.Tests/AdmissionTests.cs
Verify tests check real behavior and would fail if the implementation were broken.

⚙️ CodeRabbit configuration file

Files:

  • tests/Bws.Gui.Tests/AdmissionTests.cs
Performance is a known weak spot of these projects.

⚙️ CodeRabbit configuration file

Files:

  • src/Bws.Gui/ViewModels/Sentences.cs
  • tests/Bws.Gui.Tests/AdmissionTests.cs
Applies only to code that builds or styles a GUI.

⚙️ CodeRabbit configuration file

Files:

  • src/Bws.Gui/ViewModels/Sentences.cs
  • tests/Bws.Gui.Tests/AdmissionTests.cs
User-facing changelog.

⚙️ CodeRabbit configuration file

Files:

  • CHANGELOG.md
SECURITY, HIGH PRIORITY.

⚙️ CodeRabbit configuration file

Files:

  • src/Bws.Gui/ViewModels/Sentences.cs
  • tests/Bws.Gui.Tests/AdmissionTests.cs
C# / .NET code.

⚙️ CodeRabbit configuration file

Files:

  • src/Bws.Gui/ViewModels/Sentences.cs
  • tests/Bws.Gui.Tests/AdmissionTests.cs
Check that documentation matches the actual code in this PR: commands, flags, config keys, file paths, build steps and examples must exist.

⚙️ CodeRabbit configuration file

Files:

  • CHANGELOG.md
All code in this repository is written by an AI coding agent (Claude Code).

⚙️ CodeRabbit configuration file

Files:

  • src/Bws.Gui/ViewModels/Sentences.cs
  • CHANGELOG.md
  • tests/Bws.Gui.Tests/AdmissionTests.cs
Scope, duplication and docs: Warn if any of these is true: the PR contains significant changes not mentioned in the title/description, or mixes unrelated refactors with a feature or fix; the PR adds functionality, helpers, UI components, st...

📄 CodeRabbit inference engine (Custom checks)

Files:

  • CHANGELOG.md

📝 Walkthrough

Walkthrough

The admission logic now permits an unreadable-entry notice after all needed second-phase reads finish. Tests cover denied and successful memory reads, unread signature families, and cases where the Memory column is visible.

Changes

Partial-result notices

Layer / File(s) Summary
Admission notice condition and coverage
src/Bws.Gui/ViewModels/Sentences.cs, tests/Bws.Gui.Tests/AdmissionTests.cs, CHANGELOG.md
The notice appears when unreadable entries exist and no needed families remain unread. Tests cover denied and successful memory reads, unread signature families, and a visible Memory column. The changelog describes the notice.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: bug, ui

Merge Risk: ⚪ Minimal · up to 2e754

The window can report partial answers after required reads finish, with no remaining actionable merge risk identified.

🚥 Pre-merge checks | ✅ 14
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main behavior change: the interface admits a partial answer after the memory pass runs. It is specific and suitable for release notes.
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.
Tests For Changed Behavior ✅ Passed The PR changes the GUI admission message in Sentences.Admissions and adds four focused AdmissionTests covering refusal after the memory pass, a successful pass, an unread required family, and a sh…
No Secrets Or Debug Leftovers ✅ Passed PASS. The authoritative diff changes only CHANGELOG.md, Sentences.cs, and AdmissionTests.cs. It adds no CLAUDE.md, CLAUDE.local.md, AGENTS.md, .claude/, or .env files. Added lines contain no credentia…
No Hardcoded Ui Styling ✅ Passed The PR changes src/Bws.Gui/ViewModels/Sentences.cs, tests, and CHANGELOG.md. The source change updates admission logic only. It adds no XAML, WPF code-behind, controls, styles, templates, or liter…
No Obvious Performance Problems ✅ Passed The production change only replaces one boolean predicate in Sentences.Admissions: (needs & ~have) == ExtraRead.None. It adds no I/O, collection traversal, per-item work, timers, UI collection upd…
Desktop Robustness ✅ Passed PASS: The authoritative diff changes only the Sentences.Admissions condition, adds coverage, and updates the changelog. The production change only evaluates unreadable, needs, and have; it doe…
Safe File Parsing ✅ Passed The PR changes only admission logic, tests, and changelog text. The changed source and test files contain no file parsing, serialization, archive, path, or export APIs. The safe file parsing check is …
System Changes Are Reversible ✅ Passed PASS: The authoritative diff changes only GUI admission-note logic, tests, and CHANGELOG.md. It does not add or change code that modifies network filters, proxies, firewalls, system time, process hook…
Clear User-Facing Text ✅ Passed No unclear user-facing text was introduced. The PR changes when the existing partial-result notice is displayed; both GUI resource strings are unchanged and state what could not be read and the possib…
No Resource Leaks ✅ Passed No resource leak is introduced. The production change only changes the Sentences.Admissions condition. The new test doubles implement IProcessMemoryReader and return value objects; they do not all…
Scope, Duplication And Docs ✅ Passed The PR is scoped to the stated GUI admission fix. The production diff changes only the existing Sentences.Admissions condition, and the added tests cover the described cases. It uses the existing `E…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR

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

@coderabbitai coderabbitai Bot added bug Something isn't working ui labels Sep 23, 2026
@donislawdev
donislawdev merged commit 720dbd7 into main Sep 23, 2026
8 checks passed
@donislawdev
donislawdev deleted the fix/partial-notice-after-extra-read branch September 23, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant