You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Unable to trigger custom agent "Code Reviewer". You have run out of credits 😔
Please upgrade your plan or buy additional credits from the subscription page.
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
Updated GitHub Actions reusable-workflow pins; added MainWindow infrastructure (UI-thread marshaling, fire-and-forget event runner, search cancellation state); hardened discovery, parsing, and SQLite repository I/O; refactored session-loading and UI event flows; and expanded WPF and storage tests to cover these behaviors.
Changes
Cohort / File(s)
Summary
Workflow version pins \.github/workflows/codecov-analytics.yml, \.github/workflows/quality-zero-gate.yml, \.github/workflows/quality-zero-platform.yml
Updated reusable-workflow uses commit pins and matching platform_ref inputs: codecov-analytics.yml → 977a49739e43a4c752d8067e15020e3f06934ac7; the other two workflows → 1ac3b68d3b8289dc46a2dd75007f5eb1e3d44e09.
MainWindow infra & UI flow src/CodexSessionManager.App/MainWindow.Infrastructure.cs (new), src/CodexSessionManager.App/MainWindow.SessionOperations.cs, src/CodexSessionManager.App/MainWindow.xaml.cs
Added RunOnUiThreadAsync/RunOnUiThreadValueAsync, RunEventTask, SearchCancellationState; moved event handlers to RunEventTask; tightened null guards; changed ProcessStarter signature; adjusted cancellation lifecycle and UI-update safety.
Refactored DB access: new synchronous OpenConnection, centralized ExecuteReaderAsync/ExecuteNonQueryAsync/ExecuteCommandAsync helpers, stricter RequireConnection/RequireSession checks, and hardened handling of missing/null copies and inputs.
Pragma suppressions (S3990) multiple production & test files (many under src/... and tests/...)
Added file-scoped #pragma warning disable S3990 broadly across many source and test files to silence a Codacy/Sonar false positive related to CLS compliance.
Expanded WPF STA test coverage into partial classes and multiple new files; added many reflection targets and tests for RunEventTask, RunOnUiThread, search cancellation, UI handlers, sqlite description helpers, lifecycle, and threading behaviors.
🐰 I hopped through code with careful paws,
I chased the nulls and tightened laws,
I marshalled tasks to UI light,
I parsed the files well into night,
Tests wag their tails — the build applause!
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name
Status
Explanation
Title check
✅ Passed
The title accurately summarizes the main objective of tightening quality gates and improving null-safety throughout the codebase.
Description check
✅ Passed
The description covers the main objectives and provides a clear summary section, but lacks detail in the Verification section regarding specific test coverage.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch codex/strict-zero-csm
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.
We reviewed changes in 2415539...1228e2a on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
The reason will be displayed to describe this comment to others. Learn more.
Variable 'disposableConnection' is declared but not used
A variable is declared but not used within its scope.
Unused variables can clutter the codebase, reduce readability, and potentially indicate logical errors or incomplete implementations.
If you intend to keep a variable unused intentionally, you can use an underscore (_) as the variable name.
The reason will be displayed to describe this comment to others. Learn more.
Variable 'disposableConnection' is declared but not used
A variable is declared but not used within its scope.
Unused variables can clutter the codebase, reduce readability, and potentially indicate logical errors or incomplete implementations.
If you intend to keep a variable unused intentionally, you can use an underscore (_) as the variable name.
The reason will be displayed to describe this comment to others. Learn more.
Variable 'disposableConnection' is declared but not used
A variable is declared but not used within its scope.
Unused variables can clutter the codebase, reduce readability, and potentially indicate logical errors or incomplete implementations.
If you intend to keep a variable unused intentionally, you can use an underscore (_) as the variable name.
The reason will be displayed to describe this comment to others. Learn more.
Variable 'disposableConnection' is declared but not used
A variable is declared but not used within its scope.
Unused variables can clutter the codebase, reduce readability, and potentially indicate logical errors or incomplete implementations.
If you intend to keep a variable unused intentionally, you can use an underscore (_) as the variable name.
The reason will be displayed to describe this comment to others. Learn more.
Variable 'disposableConnection' is declared but not used
A variable is declared but not used within its scope.
Unused variables can clutter the codebase, reduce readability, and potentially indicate logical errors or incomplete implementations.
If you intend to keep a variable unused intentionally, you can use an underscore (_) as the variable name.
The reason will be displayed to describe this comment to others. Learn more.
`Using` block can be simplified
The using statement defines a scope at the end of which an object will be disposed. The downside is that this increases the indentation level of your code. However, with C# 8.0, you can use the new using declaration that no longer requires you to explicitly mention the braces. Although this reduces your code's indentation and nesting, the downside of this approach, however, is that the resource's lifetime may increase.
The reason will be displayed to describe this comment to others. Learn more.
Redundant lambda expression
Lambda expressions in C# are of the form (input-parameters) => expression. In case a lambda expression operates on a single parameter that is being passed to a method in its body, the arrow operator can be dropped and the entire body of the lambda can be replaced with just the name of the method.
The reason will be displayed to describe this comment to others. Learn more.
If-else statement can be simplified
Both the then and else blocks of the if statement contain only assignment expressions and these assignment expressions refer to the same identifier. Such statements can be rewritten using the ternary operator.
Wrap these multi-statement writes in a transaction.
Both methods mutate sessions and then issue follow-up writes to related tables in later statements, with cancellation checks between them. If a later command fails or the token is canceled, the catalog is left partially updated.
Also applies to: 206-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.Storage/Indexing/SessionCatalogRepository.cs` around
lines 146 - 176, Wrap the multi-statement update in a DB transaction: open a
transaction (e.g., BeginTransactionAsync) on the connection returned by
OpenConnection, attach that transaction to the SqliteCommand used with
UpsertSessionSql and ensure ReplaceCopyRowsAsync and RefreshSearchRowAsync run
inside the same transaction (modify their signatures to accept a transaction or
connection+transaction and use it for their commands), then Commit the
transaction only after all three steps (MergeExistingMetadataAsync result used,
ExecuteNonQueryAsync, ReplaceCopyRowsAsync, RefreshSearchRowAsync) succeed; on
exceptions or cancellation Rollback and rethrow. Ensure all commands use the
transaction and keep the existing cancellationToken usage.
206-224: ⚠️ Potential issue | 🟠 Major
Guard alias and notes too.
This tightened sessionId and tags, but null alias or notes still go straight into UPDATE sessions. Those columns — and combined_text — are NOT NULL, so a null metadata field becomes a runtime database error instead of an argument error.
Suggested fix
if (string.IsNullOrWhiteSpace(sessionId))
{
throw new ArgumentException(NullOrWhitespaceMessage, nameof(sessionId));
}
+ ArgumentNullException.ThrowIfNull(alias);
ArgumentNullException.ThrowIfNull(tags);
+ ArgumentNullException.ThrowIfNull(notes);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.Storage/Indexing/SessionCatalogRepository.cs` around
lines 206 - 224, SaveMetadataAsync currently validates sessionId and tags but
not alias or notes, which can result in DB NOT NULL constraint failures; add
explicit guards for alias and notes (e.g.,
ArgumentNullException.ThrowIfNull(alias, nameof(alias)) and
ArgumentNullException.ThrowIfNull(notes, nameof(notes)) or use
string.IsNullOrWhiteSpace checks) before creating the SqliteCommand so the
method throws a clear argument error rather than letting UpdateMetadataSql /
combined_text produce a runtime DB error.
🤖 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/CodexSessionManager.App/MainWindow.SessionOperations.cs`:
- Around line 137-153: Pass the incoming searchToken into repository calls so
the DB work is cancellable: change calls like
repository.ListSessionsAsync(CancellationToken.None) to
repository.ListSessionsAsync(searchToken) (and similarly for the other
repository calls referenced later), catch OperationCanceledException around the
await so canceled searches return early without touching the UI, and only call
RunOnUiThreadAsync to update _sessions and StatusTextBlock when the operation
completes successfully (i.e., not canceled).
In `@src/CodexSessionManager.Storage/Discovery/SessionDiscoveryService.cs`:
- Around line 69-76: NormalizeRootPath currently trims all trailing separators
which collapses filesystem roots like "/" to "" and "C:\" to "C:". Change the
trimming logic so lone roots are preserved: after replacing separators compute
trimmed = normalizedRootPath.TrimEnd(Path.DirectorySeparatorChar); if trimmed is
empty return Path.DirectorySeparatorChar.ToString() (to preserve "/"), and if
trimmed matches a Windows drive root pattern (e.g. matches @"^[A-Za-z]:$")
return trimmed + Path.DirectorySeparatorChar (to preserve "C:\"); otherwise
return trimmed. Update the NormalizeRootPath method to use these checks on
rootPath/normalizedRootPath.
In `@src/CodexSessionManager.Storage/Parsing/SessionJsonlParser.cs`:
- Around line 89-91: Replace unsafe calls to JsonElement.GetString() with the
safer TryGetString() in SessionJsonlParser: when reading the "timestamp" (used
in the condition involving parseState.StartedAtUtc and TryGetPropertyValue) and
when reading "text" (the textElement usage around line ~215), change the code to
call TryGetString() and handle null results (skip or treat as malformed) instead
of calling GetString() directly so malformed non-string JSON values won't throw
InvalidOperationException; keep existing parsing logic (DateTimeOffset.TryParse
for timestamp) but use the TryGetString() result as the input string.
---
Outside diff comments:
In `@src/CodexSessionManager.Storage/Indexing/SessionCatalogRepository.cs`:
- Around line 146-176: Wrap the multi-statement update in a DB transaction: open
a transaction (e.g., BeginTransactionAsync) on the connection returned by
OpenConnection, attach that transaction to the SqliteCommand used with
UpsertSessionSql and ensure ReplaceCopyRowsAsync and RefreshSearchRowAsync run
inside the same transaction (modify their signatures to accept a transaction or
connection+transaction and use it for their commands), then Commit the
transaction only after all three steps (MergeExistingMetadataAsync result used,
ExecuteNonQueryAsync, ReplaceCopyRowsAsync, RefreshSearchRowAsync) succeed; on
exceptions or cancellation Rollback and rethrow. Ensure all commands use the
transaction and keep the existing cancellationToken usage.
- Around line 206-224: SaveMetadataAsync currently validates sessionId and tags
but not alias or notes, which can result in DB NOT NULL constraint failures; add
explicit guards for alias and notes (e.g.,
ArgumentNullException.ThrowIfNull(alias, nameof(alias)) and
ArgumentNullException.ThrowIfNull(notes, nameof(notes)) or use
string.IsNullOrWhiteSpace checks) before creating the SqliteCommand so the
method throws a clear argument error rather than letting UpdateMetadataSql /
combined_text produce a runtime DB error.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b285ec59-04b3-49fb-a7fb-5095fe4c15c6
📥 Commits
Reviewing files that changed from the base of the PR and between 59be30f and bd860eb.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major
Propagate searchToken into the repository work.
These helpers only honor cancellation after SearchAsync / ListSessionsAsync finish, because the repository calls still run under CancellationToken.None. On rapid typing, superseded searches will keep doing full DB work in the background; the token only suppresses the final _sessions update. Pass searchToken through the repository calls and return on OperationCanceledException so canceled searches stop before hitting the UI.
Suggested fix
private async Task ReloadSessionsForSearchAsync(CancellationToken searchToken)
{
var repository = _repository;
if (repository is null)
{
return;
}
- var sessions = await repository.ListSessionsAsync(CancellationToken.None);- var searchCanceled = searchToken.IsCancellationRequested;+ if (searchToken.IsCancellationRequested)+ {+ return;+ }++ IReadOnlyList<IndexedLogicalSession> sessions;+ try+ {+ sessions = await repository.ListSessionsAsync(searchToken);+ }+ catch (OperationCanceledException) when (searchToken.IsCancellationRequested)+ {+ return;+ }+
await RunOnUiThreadAsync(() =>
{
- if (searchCanceled)+ if (searchToken.IsCancellationRequested)
{
return;
}
_sessions.Clear();
foreach (var session in sessions)
@@
- var hits = await repository.SearchAsync(searchQuery, CancellationToken.None);- var hitIds = hits.Select(hit => hit.SessionId).ToHashSet(StringComparer.Ordinal);- var allSessions = await repository.ListSessionsAsync(CancellationToken.None);+ if (searchToken.IsCancellationRequested)+ {+ return;+ }++ HashSet<string> hitIds;+ IReadOnlyList<IndexedLogicalSession> allSessions;+ try+ {+ hitIds = (await repository.SearchAsync(searchQuery, searchToken))+ .Select(hit => hit.SessionId)+ .ToHashSet(StringComparer.Ordinal);+ allSessions = await repository.ListSessionsAsync(searchToken);+ }+ catch (OperationCanceledException) when (searchToken.IsCancellationRequested)+ {+ return;+ }+
var visibleSessions = allSessions.Where(session => hitIds.Contains(session.SessionId)).ToArray();
- var searchCanceled = searchToken.IsCancellationRequested;
await RunOnUiThreadAsync(() =>
{
- if (searchCanceled)+ if (searchToken.IsCancellationRequested)
{
return;
}
Also applies to: 170-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.App/MainWindow.SessionOperations.cs` around lines 137
- 153, Pass the incoming searchToken into repository calls so the DB work is
cancellable: change calls like
repository.ListSessionsAsync(CancellationToken.None) to
repository.ListSessionsAsync(searchToken) (and similarly for the other
repository calls referenced later), catch OperationCanceledException around the
await so canceled searches return early without touching the UI, and only call
RunOnUiThreadAsync to update _sessions and StatusTextBlock when the operation
completes successfully (i.e., not canceled).
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major
Preserve filesystem roots during normalization.
This strips every trailing separator, so a configured root like / becomes "" and C:\ becomes C:. After that, the live/backup store paths are built relative to the current directory instead of the actual filesystem root. Please keep a lone path root intact while trimming only redundant trailing separators.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.Storage/Discovery/SessionDiscoveryService.cs` around
lines 69 - 76, NormalizeRootPath currently trims all trailing separators which
collapses filesystem roots like "/" to "" and "C:\" to "C:". Change the trimming
logic so lone roots are preserved: after replacing separators compute trimmed =
normalizedRootPath.TrimEnd(Path.DirectorySeparatorChar); if trimmed is empty
return Path.DirectorySeparatorChar.ToString() (to preserve "/"), and if trimmed
matches a Windows drive root pattern (e.g. matches @"^[A-Za-z]:$") return
trimmed + Path.DirectorySeparatorChar (to preserve "C:\"); otherwise return
trimmed. Update the NormalizeRootPath method to use these checks on
rootPath/normalizedRootPath.
if (parseState.StartedAtUtc == DateTimeOffset.MinValue
&& payload.TryGetProperty("timestamp", out var timestampElement)
&& TryGetPropertyValue(payload, "timestamp", out var timestampElement)
&& DateTimeOffset.TryParse(timestampElement.GetString() ?? string.Empty, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var parsedStartedAt))
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check its size
fd "SessionJsonlParser.cs" -type f
Repository: Prekzursil/codex-session-manager
Length of output: 246
🏁 Script executed:
# Get file size
wc -l src/CodexSessionManager.Storage/Parsing/SessionJsonlParser.cs
Repository: Prekzursil/codex-session-manager
Length of output: 141
🏁 Script executed:
# View the entire file with line numbers
cat -n src/CodexSessionManager.Storage/Parsing/SessionJsonlParser.cs
Repository: Prekzursil/codex-session-manager
Length of output: 13639
Replace unsafe direct GetString() calls with TryGetString() to handle malformed payloads gracefully.
Both timestampElement.GetString() (line 91) and textElement.GetString() (line 215) are called without checking the property's ValueKind. If "timestamp" or "text" is a number, object, or array instead of a string, GetString() throws InvalidOperationException and crashes the parser. Use the existing TryGetString() method instead, which safely returns null for non-string types, allowing malformed payloads to be skipped.
Suggested fix
- if (parseState.StartedAtUtc == DateTimeOffset.MinValue- && TryGetPropertyValue(payload, "timestamp", out var timestampElement)- && DateTimeOffset.TryParse(timestampElement.GetString() ?? string.Empty, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var parsedStartedAt))+ var timestamp = TryGetString(payload, "timestamp");+ if (parseState.StartedAtUtc == DateTimeOffset.MinValue+ && !string.IsNullOrWhiteSpace(timestamp)+ && DateTimeOffset.TryParse(timestamp, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var parsedStartedAt))
{
parseState.StartedAtUtc = parsedStartedAt;
}
...
- if (!TryGetPropertyValue(contentItem, "text", out var textElement))- {- return false;- }-- var contentText = textElement.GetString();+ var contentText = TryGetString(contentItem, "text");
if (string.IsNullOrWhiteSpace(contentText))
{
return false;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.Storage/Parsing/SessionJsonlParser.cs` around lines
89 - 91, Replace unsafe calls to JsonElement.GetString() with the safer
TryGetString() in SessionJsonlParser: when reading the "timestamp" (used in the
condition involving parseState.StartedAtUtc and TryGetPropertyValue) and when
reading "text" (the textElement usage around line ~215), change the code to call
TryGetString() and handle null results (skip or treat as malformed) instead of
calling GetString() directly so malformed non-string JSON values won't throw
InvalidOperationException; keep existing parsing logic (DateTimeOffset.TryParse
for timestamp) but use the TryGetString() result as the input string.
The reason will be displayed to describe this comment to others. Learn more.
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 `@tests/CodexSessionManager.App.Tests/MainWindowCoverageTests.cs`:
- Around line 349-380: The test is using a fixed polling loop which is
timing-sensitive; replace the polling with a deterministic signal by hooking the
StatusTextBlock.TextChanged event (or a DependencyProperty change callback) to a
TaskCompletionSource and complete it when the Text contains "Failed: boom", then
await that TaskCompletionSource with a reasonable timeout; update the test
RunEventTask_sets_status_when_action_failsAsync to attach the TextChanged
handler to the statusTextBlock (or use
DependencyPropertyDescriptor.AddValueChanged on the Text property), invoke
RunEventTaskMethod on MainWindow as before, and await the completion Task
instead of the for-loop polling before asserting.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5bc68433-99a1-4eea-af85-e9446eac4fb8
📥 Commits
Reviewing files that changed from the base of the PR and between bd860eb and 297fe9c.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major
The failure-path assertion is timing-sensitive.
Lines 370-378 only give the UI status update 500ms to land before asserting. Since hosted GitHub Actions is the verification path for this PR, that fixed polling window can make the lane flaky on slower runners even when RunEventTask is correct. Please wait on a deterministic signal for the status change instead of sleep-polling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/CodexSessionManager.App.Tests/MainWindowCoverageTests.cs` around lines
349 - 380, The test is using a fixed polling loop which is timing-sensitive;
replace the polling with a deterministic signal by hooking the
StatusTextBlock.TextChanged event (or a DependencyProperty change callback) to a
TaskCompletionSource and complete it when the Text contains "Failed: boom", then
await that TaskCompletionSource with a reasonable timeout; update the test
RunEventTask_sets_status_when_action_failsAsync to attach the TextChanged
handler to the statusTextBlock (or use
DependencyPropertyDescriptor.AddValueChanged on the Text property), invoke
RunEventTaskMethod on MainWindow as before, and await the completion Task
instead of the for-loop polling before asserting.
86-94: Increase polling timeout to reduce CI flakiness
Each wait loop currently caps at ~500ms. That’s tight for hosted runners and can cause intermittent failures in async UI tests. Prefer a shared wait helper with a longer timeout and clear failure message.
Refactor sketch
+private static async Task WaitUntilAsync(Func<bool> condition, int timeoutMs = 5000, int pollMs = 25)+{+ var start = Environment.TickCount64;+ while (!condition())+ {+ if (Environment.TickCount64 - start > timeoutMs)+ {+ throw new TimeoutException("Condition not met within timeout.");+ }+ await Task.Delay(pollMs);+ }+}
Then replace the repeated for+Task.Delay(10) blocks with await WaitUntilAsync(...).
Also applies to: 134-142, 183-192, 234-242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/CodexSessionManager.App.Tests/MainWindowEventCoverageTests.cs` around
lines 86 - 94, Replace the ad-hoc polling loops in MainWindowEventCoverageTests
(e.g., the loop that checks GetNamedField<TextBlock>(window,
"ThreadNameTextBlock").Text == "Handler Load") with a shared async wait helper
like WaitUntilAsync(predicate, timeout, failureMessage); implement
WaitUntilAsync to poll the predicate at a short interval (e.g., 50ms) with a
longer default timeout (e.g., several seconds) and throw an informative
exception on timeout, then update every similar loop (the blocks around the
other mentioned ranges) to call await WaitUntilAsync(() =>
GetNamedField<TextBlock>(window, "ThreadNameTextBlock").Text == "Handler Load",
TimeSpan.FromSeconds(5), "Timed out waiting for ThreadNameTextBlock to become
'Handler Load'") or an equivalent message that references the control and
expected value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/CodexSessionManager.App.Tests/MainWindowEventCoverageTests.cs`:
- Around line 220-229: The test references unresolved types MaintenanceExecutor
and MaintenanceExecutionResult; fix by importing or fully-qualifying the
namespace where those types are actually declared (or replace them with the
matching types used by MainWindow) so the compiler can find them; update the
using directives at the top of MainWindowEventCoverageTests or change the
constructors/calls to use the App's actual field/property types (referencing
MaintenanceExecutorField, SetProvider("MaintenanceRunner", ...) and the
MaintenancePreview/MaintenanceExecutionResult usage) to restore compilation.
---
Nitpick comments:
In `@tests/CodexSessionManager.App.Tests/MainWindowEventCoverageTests.cs`:
- Around line 86-94: Replace the ad-hoc polling loops in
MainWindowEventCoverageTests (e.g., the loop that checks
GetNamedField<TextBlock>(window, "ThreadNameTextBlock").Text == "Handler Load")
with a shared async wait helper like WaitUntilAsync(predicate, timeout,
failureMessage); implement WaitUntilAsync to poll the predicate at a short
interval (e.g., 50ms) with a longer default timeout (e.g., several seconds) and
throw an informative exception on timeout, then update every similar loop (the
blocks around the other mentioned ranges) to call await WaitUntilAsync(() =>
GetNamedField<TextBlock>(window, "ThreadNameTextBlock").Text == "Handler Load",
TimeSpan.FromSeconds(5), "Timed out waiting for ThreadNameTextBlock to become
'Handler Load'") or an equivalent message that references the control and
expected value.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9b4e36b-c079-464b-903f-e5e4558d3ba4
📥 Commits
Reviewing files that changed from the base of the PR and between 297fe9c and 2d4231f.
The reason will be displayed to describe this comment to others. Learn more.
Object created is dropped immediately
An object was instantiated but was neither assigned to a variable nor passed as an argument and was dropped immediately.
This is likely a mistake. Consider utilizing the object appropriately.
If the object is not required, remove the instantiation.
If the object was instantiated because the constructor has side-effects, it is a bad design pattern. Instead, consider moving
the side-effects to a separate method.
Escape assistantText when building the JSONL fixture.
assistantText is interpolated directly into JSON here. Values like C:\repo\file.txt get written with JSON escape sequences (\r, \f), so the parser never sees the intended content and these discovery tests stop exercising realistic payloads.
Verify each finding against the current code and only fix it if needed.
In `@tests/CodexSessionManager.Storage.Tests/StorageCoverageExpansionTests.cs`
around lines 446 - 454, The test helper WriteAssistantSessionAsync constructs
JSONL by interpolating assistantText directly which breaks when assistantText
contains backslashes or control characters; change it to produce valid JSON by
JSON-escaping the assistant content instead of raw string interpolation — e.g.
build the session payload as an object (or at minimum call a JSON
serializer/escaper on assistantText) and serialize each record using
JsonSerializer.Serialize (refer to WriteAssistantSessionAsync and the
response_item payload) so the assistantText is properly escaped in the output
file.
Both search paths still run SearchAsync/ListSessionsAsync under CancellationToken.None, so superseded keystrokes keep doing full DB work after cancellation. searchCanceled is also captured before the dispatcher hop, which leaves a race where a just-canceled search can still repaint the UI.
Suggested fix
private async Task ReloadSessionsForSearchAsync(CancellationToken searchToken)
{
var repository = _repository;
if (repository is null)
{
return;
}
- var sessions = await repository.ListSessionsAsync(CancellationToken.None);- var searchCanceled = IsSearchCanceled(searchToken);+ IReadOnlyList<IndexedLogicalSession> sessions;+ try+ {+ sessions = await repository.ListSessionsAsync(searchToken);+ }+ catch (OperationCanceledException) when (searchToken.IsCancellationRequested)+ {+ return;+ }+
await RunOnUiThreadAsync(() =>
{
- if (searchCanceled)+ if (searchToken.IsCancellationRequested)
{
return;
}
@@
- var hits = await repository.SearchAsync(searchQuery, CancellationToken.None);- var hitIds = hits.Select(hit => hit.SessionId).ToHashSet(StringComparer.Ordinal);- var allSessions = await repository.ListSessionsAsync(CancellationToken.None);+ HashSet<string> hitIds;+ IReadOnlyList<IndexedLogicalSession> allSessions;+ try+ {+ hitIds = (await repository.SearchAsync(searchQuery, searchToken))+ .Select(hit => hit.SessionId)+ .ToHashSet(StringComparer.Ordinal);+ allSessions = await repository.ListSessionsAsync(searchToken);+ }+ catch (OperationCanceledException) when (searchToken.IsCancellationRequested)+ {+ return;+ }+
var visibleSessions = allSessions.Where(session => hitIds.Contains(session.SessionId)).ToArray();
- var searchCanceled = IsSearchCanceled(searchToken);
await RunOnUiThreadAsync(() =>
{
- if (searchCanceled)+ if (searchToken.IsCancellationRequested)
{
return;
}
Also applies to: 179-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.App/MainWindow.SessionOperations.cs` around lines 146
- 152, The repository calls (repository.ListSessionsAsync / SearchAsync) are
using CancellationToken.None and IsSearchCanceled is evaluated before the
dispatcher hop, so cancelled searches still do DB work and can race to repaint;
change the repository calls to pass the provided searchToken and move the
IsSearchCanceled(searchToken) check inside the RunOnUiThreadAsync callback
(i.e., evaluate cancellation after awaiting the UI dispatcher) so you abort UI
updates for a cancelled search; apply the same change for both occurrences
around ListSessionsAsync and SearchAsync.
Preserve lone filesystem roots during normalization.
This still trims lone roots down to non-roots ("/" → "", "C:\" → "C:"). After that, the derived store paths are no longer rooted and can resolve relative to the current working directory.
Suggested fix
private static string NormalizeRootPath(string rootPath)
{
var rawRootPath = rootPath ?? throw new ArgumentNullException(nameof(rootPath));
var normalizedRootPath = rawRootPath;
normalizedRootPath = normalizedRootPath.Replace('\\', Path.DirectorySeparatorChar);
normalizedRootPath = normalizedRootPath.Replace('/', Path.DirectorySeparatorChar);
- return normalizedRootPath.TrimEnd(Path.DirectorySeparatorChar);+ var trimmed = normalizedRootPath.TrimEnd(Path.DirectorySeparatorChar);+ if (trimmed.Length == 0)+ {+ return Path.DirectorySeparatorChar.ToString();+ }++ if (trimmed.Length == 2+ && char.IsLetter(trimmed[0])+ && trimmed[1] == Path.VolumeSeparatorChar)+ {+ return trimmed + Path.DirectorySeparatorChar;+ }++ return trimmed;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.Storage/Discovery/SessionDiscoveryService.cs` around
lines 65 - 72, NormalizeRootPath is incorrectly trimming lone filesystem roots
(e.g., "/" or "C:\") to non-root values; fix by detecting when the input is a
filesystem root via Path.GetPathRoot(rawRootPath) and returning that root as-is
(preserving its separator) instead of applying TrimEnd; otherwise proceed with
the existing replacement + TrimEnd logic. Use the existing
rawRootPath/normalizedRootPath variables and Path.GetPathRoot to implement the
check inside NormalizeRootPath.
48-70: Fire-and-forget task pattern is intentional but consider logging.
The discarded task (_ = RunEventTaskCoreAsync()) is a valid fire-and-forget pattern for event handlers. The exception handling catches errors and reports them via StatusTextBlock. However, for observability, consider adding structured logging of the exception alongside the UI status update.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.App/MainWindow.Infrastructure.cs` around lines 48 -
70, RunEventTask currently fire-and-forgets RunEventTaskCoreAsync and updates
StatusTextBlock on exceptions but doesn't log them; modify
RunEventTaskCoreAsync's catch block to also write a structured error to your
application's logger (e.g., call logger.LogError(ex, "{FailurePrefix} failed",
failurePrefix) or equivalent), referencing the RunEventTask and
RunEventTaskCoreAsync methods and the failurePrefix parameter so the exception
is both shown in the UI via StatusTextBlock and recorded in logs for
observability.
🤖 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/CodexSessionManager.App/MainWindow.xaml.cs`:
- Line 316: The current target collection uses
selectedSessions.SelectMany(static session => session.PhysicalCopies ?? [])
which drops sessions when PhysicalCopies is null; update the SelectMany so that
when session.PhysicalCopies is null it falls back to the session.PreferredCopy
(if non-null) instead of returning zero items. Modify the lambda used to build
targets in MainWindow.xaml.cs to yield session.PhysicalCopies when present,
otherwise yield a single-item sequence containing session.PreferredCopy (or an
empty sequence if PreferredCopy is also null), ensuring the confirmation count
and preview include the preferred copy.
In `@src/CodexSessionManager.Storage/Maintenance/MaintenanceExecutor.cs`:
- Around line 36-47: The ValidateTypedConfirmation method throws when
preview.RequiresTypedConfirmation is false; change the logic to return early
when no typed confirmation is required: in
ValidateTypedConfirmation(MaintenancePreview preview, string typedConfirmation)
first check if preview.RequiresTypedConfirmation is false and simply return,
otherwise validate that typedConfirmation is not null/whitespace and that it
equals preview.RequiredTypedConfirmation using StringComparison.Ordinal,
throwing the existing InvalidOperationException messages only in the required
case.
In `@tests/CodexSessionManager.App.Tests/MainWindowActionCoverageTests.cs`:
- Around line 449-450: The file is missing the final closing brace for the
MainWindowCoverageTests class; add a single '}' after the last test method to
close the MainWindowCoverageTests class declaration (and verify the surrounding
braces so the namespace and file-level braces remain balanced).
In `@tests/CodexSessionManager.App.Tests/MainWindowLifecycleCoverageTests.cs`:
- Around line 374-375: The class MainWindowCoverageTests is missing its final
closing brace causing build failures; add a single closing '}' to terminate the
MainWindowCoverageTests class declaration (which begins with the class keyword
MainWindowCoverageTests) after the last test method so the class scope is
properly closed.
In `@tests/CodexSessionManager.App.Tests/MainWindowSessionStateCoverageTests.cs`:
- Around line 424-425: The class MainWindowCoverageTests is missing its final
closing brace causing the build to fail; add a single closing '}' to terminate
the MainWindowCoverageTests class (after the existing last test method) so the
class declaration and all methods are properly closed.
In `@tests/CodexSessionManager.App.Tests/MainWindowThreadingCoverageTests.cs`:
- Around line 443-444: The class MainWindowCoverageTests is missing its final
closing brace which is causing the build failure; add a single closing '}' to
terminate the MainWindowCoverageTests class after the last test method (the
method ending just before the shown diff) so the class declaration
(MainWindowCoverageTests) is properly closed and the file compiles.
---
Outside diff comments:
In `@tests/CodexSessionManager.Storage.Tests/StorageCoverageExpansionTests.cs`:
- Around line 446-454: The test helper WriteAssistantSessionAsync constructs
JSONL by interpolating assistantText directly which breaks when assistantText
contains backslashes or control characters; change it to produce valid JSON by
JSON-escaping the assistant content instead of raw string interpolation — e.g.
build the session payload as an object (or at minimum call a JSON
serializer/escaper on assistantText) and serialize each record using
JsonSerializer.Serialize (refer to WriteAssistantSessionAsync and the
response_item payload) so the assistantText is properly escaped in the output
file.
---
Duplicate comments:
In `@src/CodexSessionManager.App/MainWindow.SessionOperations.cs`:
- Around line 146-152: The repository calls (repository.ListSessionsAsync /
SearchAsync) are using CancellationToken.None and IsSearchCanceled is evaluated
before the dispatcher hop, so cancelled searches still do DB work and can race
to repaint; change the repository calls to pass the provided searchToken and
move the IsSearchCanceled(searchToken) check inside the RunOnUiThreadAsync
callback (i.e., evaluate cancellation after awaiting the UI dispatcher) so you
abort UI updates for a cancelled search; apply the same change for both
occurrences around ListSessionsAsync and SearchAsync.
In `@src/CodexSessionManager.Storage/Discovery/SessionDiscoveryService.cs`:
- Around line 65-72: NormalizeRootPath is incorrectly trimming lone filesystem
roots (e.g., "/" or "C:\") to non-root values; fix by detecting when the input
is a filesystem root via Path.GetPathRoot(rawRootPath) and returning that root
as-is (preserving its separator) instead of applying TrimEnd; otherwise proceed
with the existing replacement + TrimEnd logic. Use the existing
rawRootPath/normalizedRootPath variables and Path.GetPathRoot to implement the
check inside NormalizeRootPath.
---
Nitpick comments:
In `@src/CodexSessionManager.App/MainWindow.Infrastructure.cs`:
- Around line 48-70: RunEventTask currently fire-and-forgets
RunEventTaskCoreAsync and updates StatusTextBlock on exceptions but doesn't log
them; modify RunEventTaskCoreAsync's catch block to also write a structured
error to your application's logger (e.g., call logger.LogError(ex,
"{FailurePrefix} failed", failurePrefix) or equivalent), referencing the
RunEventTask and RunEventTaskCoreAsync methods and the failurePrefix parameter
so the exception is both shown in the UI via StatusTextBlock and recorded in
logs for observability.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45d0dd65-f4ad-4a4d-adeb-0c52f2030d06
📥 Commits
Reviewing files that changed from the base of the PR and between 2d4231f and 447a540.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟡 Minor
Fallback to PreferredCopy instead of dropping the selection.
If PhysicalCopies is unexpectedly null, this builds a preview for zero files even though the selected session still has a usable preferred copy. That undercounts the confirmation text and silently skips work.
Suggested fix
- var targets = selectedSessions.SelectMany(static session => session.PhysicalCopies ?? []).ToArray();+ var targets = selectedSessions+ .SelectMany(session => session.PhysicalCopies ?? [GetRequiredPreferredCopy(session)])+ .ToArray();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.App/MainWindow.xaml.cs` at line 316, The current
target collection uses selectedSessions.SelectMany(static session =>
session.PhysicalCopies ?? []) which drops sessions when PhysicalCopies is null;
update the SelectMany so that when session.PhysicalCopies is null it falls back
to the session.PreferredCopy (if non-null) instead of returning zero items.
Modify the lambda used to build targets in MainWindow.xaml.cs to yield
session.PhysicalCopies when present, otherwise yield a single-item sequence
containing session.PreferredCopy (or an empty sequence if PreferredCopy is also
null), ensuring the confirmation count and preview include the preferred copy.
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major
Detect a bare sessions_backup root as a backup store.
When RootPath is exactly sessions_backup, this branch never matches because the suffix check includes a leading separator. The code then looks for session_index.jsonl inside the backup folder instead of its parent workspace, so thread names fall back unnecessarily.
Suggested fix
- var backupWorkspaceRoot = Path.GetDirectoryName(normalizedRoot);+ var backupWorkspaceRoot = Path.GetDirectoryName(Path.GetFullPath(normalizedRoot));
var normalizedBackupWorkspaceRoot = string.IsNullOrWhiteSpace(backupWorkspaceRoot) ? normalizedRoot : backupWorkspaceRoot;
@@
- if (storeKind == SessionStoreKind.Backup- && normalizedRoot.EndsWith(- $"{Path.DirectorySeparatorChar}sessions_backup",- StringComparison.OrdinalIgnoreCase))+ if (storeKind == SessionStoreKind.Backup+ && string.Equals(+ Path.GetFileName(normalizedRoot),+ "sessions_backup",+ StringComparison.OrdinalIgnoreCase))
{
return new KnownSessionStore(
normalizedBackupWorkspaceRoot,
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue | 🟠 Major
Return early when confirmation isn't required.
As written, RequiresTypedConfirmation == false still throws "Typed confirmation is required." and prevents execution of previews that intentionally skip typed confirmation.
Suggested fix
private static void ValidateTypedConfirmation(MaintenancePreview preview, string typedConfirmation)
{
- if (!preview.RequiresTypedConfirmation || string.IsNullOrWhiteSpace(typedConfirmation))+ if (!preview.RequiresTypedConfirmation)+ {+ return;+ }++ if (string.IsNullOrWhiteSpace(typedConfirmation))
{
throw new InvalidOperationException("Typed confirmation is required.");
}
if (!string.Equals(preview.RequiredTypedConfirmation, typedConfirmation, StringComparison.Ordinal))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.Storage/Maintenance/MaintenanceExecutor.cs` around
lines 36 - 47, The ValidateTypedConfirmation method throws when
preview.RequiresTypedConfirmation is false; change the logic to return early
when no typed confirmation is required: in
ValidateTypedConfirmation(MaintenancePreview preview, string typedConfirmation)
first check if preview.RequiresTypedConfirmation is false and simply return,
otherwise validate that typedConfirmation is not null/whitespace and that it
equals preview.RequiredTypedConfirmation using StringComparison.Ordinal,
throwing the existing InvalidOperationException messages only in the required
case.
Cancelled searches still keep doing repository work, and the UI cancel check is racey.
Line 140 and Line 173 still pass CancellationToken.None, so superseded searches continue hitting the repository after a new token replaces them. On top of that, Lines 141 and 177 snapshot cancellation beforeRunOnUiThreadAsync, so a cancellation arriving after that snapshot can still let stale results update _sessions.
Suggested fix
- var sessions = await repository.ListSessionsAsync(CancellationToken.None);- var searchCanceled = IsSearchCanceled(searchToken);+ if (searchToken.IsCancellationRequested)+ {+ return;+ }++ IReadOnlyList<IndexedLogicalSession> sessions;+ try+ {+ sessions = await repository.ListSessionsAsync(searchToken);+ }+ catch (OperationCanceledException) when (searchToken.IsCancellationRequested)+ {+ return;+ }
await RunOnUiThreadAsync(() =>
{
- if (searchCanceled)+ if (searchToken.IsCancellationRequested)
{
return;
}
@@
- var hits = await repository.SearchAsync(searchQuery, CancellationToken.None);- var hitIds = hits.Select(hit => hit.SessionId).ToHashSet(StringComparer.Ordinal);- var allSessions = await repository.ListSessionsAsync(CancellationToken.None);+ if (searchToken.IsCancellationRequested)+ {+ return;+ }++ HashSet<string> hitIds;+ IReadOnlyList<IndexedLogicalSession> allSessions;+ try+ {+ hitIds = (await repository.SearchAsync(searchQuery, searchToken))+ .Select(hit => hit.SessionId)+ .ToHashSet(StringComparer.Ordinal);+ allSessions = await repository.ListSessionsAsync(searchToken);+ }+ catch (OperationCanceledException) when (searchToken.IsCancellationRequested)+ {+ return;+ }
var visibleSessions = allSessions.Where(session => hitIds.Contains(session.SessionId)).ToArray();
- var searchCanceled = IsSearchCanceled(searchToken);
await RunOnUiThreadAsync(() =>
{
- if (searchCanceled)+ if (searchToken.IsCancellationRequested)
{
return;
}
Also applies to: 173-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.App/MainWindow.SessionOperations.cs` around lines 140
- 145, Replace the use of CancellationToken.None and the pre-snapshotted
searchCanceled boolean by passing the actual search token into
repository.ListSessionsAsync and performing the cancellation check inside the UI
update callback: call repository.ListSessionsAsync(searchToken) instead of
CancellationToken.None, remove the early IsSearchCanceled(searchToken) snapshot,
and inside the lambda passed to RunOnUiThreadAsync check
searchToken.IsCancellationRequested (or IsSearchCanceled(searchToken)) before
mutating _sessions so stale results cannot overwrite state; apply the same
changes to both occurrences surrounding repository.ListSessionsAsync /
RunOnUiThreadAsync in MainWindow.SessionOperations.cs.
24-25: The normalizedFilePath variable provides no actual normalization.
The variable is introduced but simply assigned the input filePath without any normalization (e.g., Path.GetFullPath). If normalization is intended for consistency with error messages and parsed output, apply it; otherwise, consider removing the intermediate variable.
Optional: Apply actual normalization
- var normalizedFilePath = filePath;+ var normalizedFilePath = Path.GetFullPath(filePath);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/CodexSessionManager.Storage/Parsing/SessionJsonlParser.cs` around lines
24 - 25, The temporary variable normalizedFilePath currently just copies
filePath without normalization; either remove normalizedFilePath and use
filePath directly, or actually normalize it (e.g., call Path.GetFullPath or
Path.GetFullPath(Path.Combine(Directory.GetCurrentDirectory(), filePath)) before
passing to File.ReadAllLinesAsync) so error messages and parsed output show a
consistent absolute path; update the usage in the method (where
normalizedFilePath is defined and passed to File.ReadAllLinesAsync) accordingly
and keep only one of the two approaches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/CodexSessionManager.Storage.Tests/StorageGuardClauseTests.cs`:
- Line 147: The test asserts that TryExtractExitCode throws
ArgumentNullException via AssertInner<ArgumentNullException>(() =>
TryExtractExitCodeMethod.Invoke(null, [null!, 0])), but TryExtractExitCode is
designed to return false for null/whitespace input (see parser behavior), so
remove this assertion line; update StorageGuardClauseTests.cs to delete the
AssertInner<ArgumentNullException>(...) invocation referencing
TryExtractExitCodeMethod.Invoke and, if needed, replace it with an assertion
that Invoke returns false or does not throw to reflect intended behavior.
---
Duplicate comments:
In `@src/CodexSessionManager.App/MainWindow.SessionOperations.cs`:
- Around line 140-145: Replace the use of CancellationToken.None and the
pre-snapshotted searchCanceled boolean by passing the actual search token into
repository.ListSessionsAsync and performing the cancellation check inside the UI
update callback: call repository.ListSessionsAsync(searchToken) instead of
CancellationToken.None, remove the early IsSearchCanceled(searchToken) snapshot,
and inside the lambda passed to RunOnUiThreadAsync check
searchToken.IsCancellationRequested (or IsSearchCanceled(searchToken)) before
mutating _sessions so stale results cannot overwrite state; apply the same
changes to both occurrences surrounding repository.ListSessionsAsync /
RunOnUiThreadAsync in MainWindow.SessionOperations.cs.
---
Nitpick comments:
In `@src/CodexSessionManager.Storage/Parsing/SessionJsonlParser.cs`:
- Around line 24-25: The temporary variable normalizedFilePath currently just
copies filePath without normalization; either remove normalizedFilePath and use
filePath directly, or actually normalize it (e.g., call Path.GetFullPath or
Path.GetFullPath(Path.Combine(Directory.GetCurrentDirectory(), filePath)) before
passing to File.ReadAllLinesAsync) so error messages and parsed output show a
consistent absolute path; update the usage in the method (where
normalizedFilePath is defined and passed to File.ReadAllLinesAsync) accordingly
and keep only one of the two approaches.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb5d1c30-85d9-4a70-b397-0da36e35c765
📥 Commits
Reviewing files that changed from the base of the PR and between 447a540 and 4525db0.
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
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
Verification
Summary by CodeRabbit
Bug Fixes
Chores
Tests