feat: progress display redesign#28
Merged
Merged
Conversation
- Replace FileScannedEvent(long TotalFiles) with per-file (RelativePath, FileSize); add ScanCompleteEvent and TarBundleStartedEvent - Add OnHashQueueReady/OnUploadQueueReady callbacks to ArchiveOptions; wire queue depth in CliBuilder - Redesign ProgressState: simplified FileState enum, TrackedTar/TarState, new aggregate fields (FilesScanned, FilesUnique, queue depths) - Rewrite all notification handlers: FileScanned, ScanComplete, TarBundleStarted, TarEntryAdded, ChunkUploading (dual-lookup), TarBundleSealing/Uploaded - Redesign BuildArchiveDisplay: live scanning counter, hashing/uploading queue depth, TAR bundle lines, filter files to Hashing/Uploading only - Fix Spectre markup: escape queue-depth brackets as [[N pending]] to avoid style-tag parse errors - Fully rewrite ProgressTests (103 tests, all passing)
Contributor
📝 WalkthroughWalkthroughThis pull request redesigns the archive progress display pipeline: it changes scanning from a batch total to per-file Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Archive Client
participant Pipeline as Archive Pipeline
participant Scanner as File Scanner
participant Hash as Hash Stage
participant TarBuilder as TAR Builder
participant Upload as Upload Stage
participant CLI as CLI Progress
Client->>Pipeline: Start archive operation
Pipeline->>Scanner: Begin enumeration
loop per file discovered
Scanner->>Pipeline: file discovered (path,size)
Pipeline->>CLI: FileScannedEvent(path, size)
CLI->>CLI: FilesScanned++, BytesScanned += size
end
Scanner->>Pipeline: enumeration complete
Pipeline->>CLI: ScanCompleteEvent(totalFiles, totalBytes)
CLI->>CLI: ScanComplete = true
par hashing and TAR building
loop per file
Pipeline->>Hash: schedule hash
Hash->>CLI: FileHashedEvent(contentHash)
CLI->>CLI: mark TrackedFile state = Hashed
end
and
loop per TAR bundle
TarBuilder->>CLI: TarBundleStartedEvent()
TarBuilder->>CLI: TarEntryAddedEvent(...)
CLI->>CLI: remove small files from TrackedFiles, update TrackedTar.accumulated
TarBuilder->>CLI: TarBundleSealingEvent(tarHash,totalBytes)
CLI->>CLI: TrackedTar.state = Sealing, set TarHash
end
end
par upload flows
loop large-file uploads
Upload->>CLI: ChunkUploadingEvent(contentHash,...)
CLI->>CLI: find TrackedFile via contentHash → set Uploading, reset BytesProcessed
Upload->>CLI: ProgressStream updates
CLI->>CLI: TrackedFile.BytesProcessed += delta
end
and TAR uploads
loop per TAR upload
Upload->>CLI: ChunkUploadingEvent(tarHash,...)
CLI->>CLI: find TrackedTar by TarHash → set Uploading
Upload->>CLI: ProgressStream updates
CLI->>CLI: TrackedTar.BytesUploaded += delta
Upload->>CLI: TarBundleUploadedEvent(tarHash)
CLI->>CLI: remove TrackedTar
end
end
CLI->>Client: Render multi-section display (Scanning / Hashing / Uploading + per-file and TAR rows)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #28 +/- ##
==========================================
+ Coverage 81.52% 82.04% +0.52%
==========================================
Files 27 27
Lines 2890 3002 +112
Branches 330 354 +24
==========================================
+ Hits 2356 2463 +107
+ Misses 458 457 -1
- Partials 76 82 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Arius.Core/Archive/ArchivePipelineHandler.cs (1)
173-212:⚠️ Potential issue | 🔴 CriticalPropagate hash-stage exceptions to the channel instead of suppressing them with a normal EOF completion.
Parallel.ForEachAsync(...).ContinueWith(_ => hashedChannel.Writer.Complete(), CancellationToken.None)executes its continuation regardless of success or failure. When the parent task faults,Complete()is called without an exception argument, which signals normal EOF to downstream readers. This allows the dedup and upload stages to process incomplete data and return success, creating a silent data corruption path.
hashTaskis never awaited in the pipeline completion (lines 571–576), so exceptions are never exposed.The fix is to check the task status and propagate exceptions:
.ContinueWith(t => { if (t.IsFaulted) hashedChannel.Writer.Complete(t.Exception); else hashedChannel.Writer.Complete(); }, CancellationToken.None);Also applies to: 570–576 (audit full pipeline await list).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Arius.Core/Archive/ArchivePipelineHandler.cs` around lines 173 - 212, The Parallel.ForEachAsync continuation currently always calls hashedChannel.Writer.Complete(), which suppresses exceptions and signals a normal EOF; change the continuation on the Task returned by Parallel.ForEachAsync (hashTask) to inspect the task status and call hashedChannel.Writer.Complete(exception) when the task IsFaulted (propagating t.Exception) otherwise call Complete() normally, and also ensure hashTask is included/awaited in the pipeline completion await list so any exceptions bubble up to the caller; look for Parallel.ForEachAsync, hashedChannel.Writer.Complete, and the hashTask variable to make these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/progress-display-redesign/specs/archive-pipeline/spec.md`:
- Around line 58-68: The spec requires that when CreateHashProgress or
CreateUploadProgress are null the pipeline should NOT wrap streams in
ProgressStream; update ArchivePipelineHandler to follow that: in the code paths
that currently call new Progress<long>() and wrap the FileStream or TAR upload
stream with ProgressStream, change the logic to only construct and use
ProgressStream when CreateHashProgress/CreateUploadProgress returns a non-null
IProgress<long>, otherwise pass/use the original stream directly (no-op) so
there is no ProgressStream overhead; locate references to CreateHashProgress,
CreateUploadProgress, ProgressStream and the wrapping logic in
ArchivePipelineHandler and branch accordingly.
In `@src/Arius.Cli.Tests/ProgressTests.cs`:
- Around line 739-770: Add two unit tests to cover zero-queue upload states:
create one test that instantiates ProgressState, calls
IncrementChunksUploaded(1) (or leaves as-is if first upload is in flight), sets
UploadQueueDepth = () => 0, renders CliBuilder.BuildArchiveDisplay(state) via
RenderToString and asserts the header shows the zero-queue indication (e.g.,
contains "0" or the expected "pending"/queue text for zero); create another test
that sets UploadQueueDepth = () => 0, ensures SnapshotComplete is set via
SetSnapshotComplete() and ChunksUploaded is zero (no IncrementChunksUploaded),
renders with CliBuilder.BuildArchiveDisplay(state) and asserts the header shows
the completed/empty-run indicator (e.g., the filled/open circle expected). Use
the existing ProgressState, IncrementChunksUploaded, UploadQueueDepth,
SetSnapshotComplete, CliBuilder.BuildArchiveDisplay and RenderToString symbols
to locate where to add these tests.
In `@src/Arius.Cli/CliBuilder.cs`:
- Around line 893-910: The uploading header should be considered active as soon
as an upload is in-flight (even before any chunk/tar count or queue depth
appears) — update the condition that decides the active vs grey branch to treat
the header as active when SnapshotComplete is false or when ChunksUploaded,
TarsUploaded, or UploadQueueDepth indicate work; specifically, change the if
that currently checks (state.ChunksUploaded > 0 || state.TarsUploaded > 0 ||
queueDepth > 0) to also consider state.SnapshotComplete (e.g., if
(!state.SnapshotComplete || state.ChunksUploaded > 0 || state.TarsUploaded > 0
|| queueDepth > 0)) and ensure uploadSymbol still uses state.SnapshotComplete to
show completed vs in-progress.
In `@src/Arius.Cli/ProgressHandlers.cs`:
- Around line 65-68: The current code computes bundleNumber as
state.TrackedTars.Count + 1 and hard-codes 64MB, which can reuse IDs and ignore
overridden TarTargetSize; fix by adding a monotonic counter (e.g.,
ProgressState.NextBundleId or MonotonicBundleCounter) and an effective
TarTargetSize property on ProgressState, then obtain bundleNumber by
incrementing that counter (Interlocked.Increment) and construct the TrackedTar
with state.TarTargetSize (not 64L * 1024 * 1024) before calling
state.TrackedTars.TryAdd, ensuring each new TrackedTar uses a unique,
monotonically increasing ID.
- Around line 145-156: The current branch checks
ContentHashToPath.ContainsKey(...) which can be stale; change
ProgressState.SetFileUploading(string contentHash) to return a bool that
indicates whether it actually transitioned any TrackedFiles to
FileState.Uploading, then in ProgressHandlers.cs call that method and only call
state.IncrementFilesUnique() and return when SetFileUploading(...) returns true;
if it returns false, fall through to the TAR lookup (state.TrackedTars and
TarState.Uploading) so matching tar entries can still be promoted.
---
Outside diff comments:
In `@src/Arius.Core/Archive/ArchivePipelineHandler.cs`:
- Around line 173-212: The Parallel.ForEachAsync continuation currently always
calls hashedChannel.Writer.Complete(), which suppresses exceptions and signals a
normal EOF; change the continuation on the Task returned by
Parallel.ForEachAsync (hashTask) to inspect the task status and call
hashedChannel.Writer.Complete(exception) when the task IsFaulted (propagating
t.Exception) otherwise call Complete() normally, and also ensure hashTask is
included/awaited in the pipeline completion await list so any exceptions bubble
up to the caller; look for Parallel.ForEachAsync, hashedChannel.Writer.Complete,
and the hashTask variable to make these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 037a5d35-4765-4867-9182-ee536fbe2b97
📒 Files selected for processing (13)
openspec/changes/progress-display-redesign/.openspec.yamlopenspec/changes/progress-display-redesign/design.mdopenspec/changes/progress-display-redesign/proposal.mdopenspec/changes/progress-display-redesign/specs/archive-pipeline/spec.mdopenspec/changes/progress-display-redesign/specs/cli/spec.mdopenspec/changes/progress-display-redesign/specs/progress-display/spec.mdopenspec/changes/progress-display-redesign/tasks.mdsrc/Arius.Cli.Tests/ProgressTests.cssrc/Arius.Cli/CliBuilder.cssrc/Arius.Cli/ProgressHandlers.cssrc/Arius.Cli/ProgressState.cssrc/Arius.Core/Archive/ArchiveModels.cssrc/Arius.Core/Archive/ArchivePipelineHandler.cs
Comment on lines
+58
to
+68
| #### Scenario: Core uses hash progress callback | ||
| - **WHEN** a file begins hashing and `CreateHashProgress` is not null | ||
| - **THEN** the pipeline SHALL call `CreateHashProgress(relativePath, fileSize)` and wrap the `FileStream` in a `ProgressStream` using the returned `IProgress<long>` | ||
| - **WHEN** `CreateHashProgress` is null | ||
| - **THEN** the pipeline SHALL hash the file stream directly without wrapping | ||
|
|
||
| #### Scenario: Core uses upload progress callback | ||
| - **WHEN** a chunk begins uploading and `CreateUploadProgress` is not null | ||
| - **THEN** the pipeline SHALL call `CreateUploadProgress(contentHash, size)` and use the returned `IProgress<long>` with `ProgressStream` | ||
| - **WHEN** `CreateUploadProgress` is null | ||
| - **THEN** the pipeline SHALL use a no-op `IProgress<long>` (current behavior) |
Contributor
There was a problem hiding this comment.
These SHALLs don't match the current null-callback path.
src/Arius.Core/Archive/ArchivePipelineHandler.cs still wraps both the hash stream and the TAR upload stream in ProgressStream with new Progress<long>() when the callback is null. As written, this spec promises a direct stream path and "no ProgressStream overhead", which the implementation in this PR does not do.
Also applies to: 176-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/progress-display-redesign/specs/archive-pipeline/spec.md`
around lines 58 - 68, The spec requires that when CreateHashProgress or
CreateUploadProgress are null the pipeline should NOT wrap streams in
ProgressStream; update ArchivePipelineHandler to follow that: in the code paths
that currently call new Progress<long>() and wrap the FileStream or TAR upload
stream with ProgressStream, change the logic to only construct and use
ProgressStream when CreateHashProgress/CreateUploadProgress returns a non-null
IProgress<long>, otherwise pass/use the original stream directly (no-op) so
there is no ProgressStream overhead; locate references to CreateHashProgress,
CreateUploadProgress, ProgressStream and the wrapping logic in
ArchivePipelineHandler and branch accordingly.
Comment on lines
+739
to
+770
| [Test] | ||
| public void BuildArchiveDisplay_UploadingHeader_ShowsQueueDepth_WhenNonZero() | ||
| { | ||
| var state = new ProgressState(); | ||
| state.IncrementChunksUploaded(100); | ||
| state.UploadQueueDepth = () => 3; | ||
|
|
||
| var output = RenderToString(CliBuilder.BuildArchiveDisplay(state)); | ||
| output.ShouldContain("3"); | ||
| output.ShouldContain("pending"); | ||
| } | ||
|
|
||
| [Test] | ||
| public void BuildArchiveDisplay_UploadingHeader_FilledCircle_WhenSnapshotComplete() | ||
| { | ||
| var state = new ProgressState(); | ||
| state.IncrementChunksUploaded(100); | ||
| state.SetSnapshotComplete(); | ||
|
|
||
| var output = RenderToString(CliBuilder.BuildArchiveDisplay(state)); | ||
| output.ShouldContain("●"); | ||
| } | ||
|
|
||
| [Test] | ||
| public void BuildArchiveDisplay_UploadingHeader_OpenCircle_WhenNotComplete() | ||
| { | ||
| var state = new ProgressState(); | ||
| state.IncrementChunksUploaded(100); | ||
| // SnapshotComplete NOT set | ||
|
|
||
| var output = RenderToString(CliBuilder.BuildArchiveDisplay(state)); | ||
| output.ShouldContain("○"); |
Contributor
There was a problem hiding this comment.
Add regressions for the zero-queue upload states.
These assertions only cover the header after ChunksUploaded increments. They won't catch the grey header while the first upload is already in flight, or after a fully deduped/empty run completes with SnapshotComplete and zero uploaded chunks.
🧪 Suggested cases
+ [Test]
+ public void BuildArchiveDisplay_UploadingHeader_OpenCircle_WhenUploadIsInFlight()
+ {
+ var state = new ProgressState();
+ state.AddFile("large.bin", 10_000_000);
+ state.SetFileHashed("large.bin", "hash1");
+ state.SetFileUploading("hash1");
+
+ var output = RenderToString(CliBuilder.BuildArchiveDisplay(state));
+ output.ShouldContain("Uploading");
+ output.ShouldContain("○");
+ }
+
+ [Test]
+ public void BuildArchiveDisplay_UploadingHeader_FilledCircle_WhenSnapshotCompletesWithoutUploads()
+ {
+ var state = new ProgressState();
+ state.SetSnapshotComplete();
+
+ var output = RenderToString(CliBuilder.BuildArchiveDisplay(state));
+ output.ShouldContain("Uploading");
+ output.ShouldContain("●");
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Arius.Cli.Tests/ProgressTests.cs` around lines 739 - 770, Add two unit
tests to cover zero-queue upload states: create one test that instantiates
ProgressState, calls IncrementChunksUploaded(1) (or leaves as-is if first upload
is in flight), sets UploadQueueDepth = () => 0, renders
CliBuilder.BuildArchiveDisplay(state) via RenderToString and asserts the header
shows the zero-queue indication (e.g., contains "0" or the expected
"pending"/queue text for zero); create another test that sets UploadQueueDepth =
() => 0, ensures SnapshotComplete is set via SetSnapshotComplete() and
ChunksUploaded is zero (no IncrementChunksUploaded), renders with
CliBuilder.BuildArchiveDisplay(state) and asserts the header shows the
completed/empty-run indicator (e.g., the filled/open circle expected). Use the
existing ProgressState, IncrementChunksUploaded, UploadQueueDepth,
SetSnapshotComplete, CliBuilder.BuildArchiveDisplay and RenderToString symbols
to locate where to add these tests.
Comment on lines
+893
to
+910
| // ── 6.3 Uploading header ────────────────────────────────────────────── | ||
| // Shows unique chunks uploaded, queue depth. | ||
| { | ||
| var chunksUploaded = state.ChunksUploaded; | ||
| var tarsUploaded = state.TarsUploaded; | ||
| var queueDepth = state.UploadQueueDepth?.Invoke() ?? 0; | ||
|
|
||
| // Hashing | ||
| if (totalFiles.HasValue && filesHashed >= totalFiles.Value) | ||
| lines.Add(new Markup($" [green]●[/] Hashing [dim]{filesHashed}/{totalFiles.Value}[/]")); | ||
| else if (totalFiles.HasValue) | ||
| lines.Add(new Markup($" [yellow]○[/] Hashing [dim]{filesHashed}/{totalFiles.Value}[/]")); | ||
| else | ||
| lines.Add(new Markup($" [yellow]○[/] Hashing [dim]{filesHashed}...[/]")); | ||
|
|
||
| // Uploading | ||
| var chunksUploaded = state.ChunksUploaded; | ||
| var totalChunks = state.TotalChunks; | ||
| var tarsUploaded = state.TarsUploaded; | ||
| if (totalChunks.HasValue && chunksUploaded >= totalChunks.Value && chunksUploaded > 0) | ||
| lines.Add(new Markup($" [green]●[/] Uploading [dim]{chunksUploaded}/{totalChunks.Value} chunks[/]")); | ||
| else if (totalChunks.HasValue) | ||
| lines.Add(new Markup($" [yellow]○[/] Uploading [dim]{chunksUploaded}/{totalChunks.Value} chunks[/]")); | ||
| else if (chunksUploaded > 0 || tarsUploaded > 0) | ||
| lines.Add(new Markup($" [yellow]○[/] Uploading [dim]{chunksUploaded} chunks...[/]")); | ||
| else | ||
| lines.Add(new Markup(" [grey] Uploading[/]")); | ||
| if (chunksUploaded > 0 || tarsUploaded > 0 || queueDepth > 0) | ||
| { | ||
| var queuePart = queueDepth > 0 ? $" [dim][[{queueDepth} pending]][/]" : string.Empty; | ||
| var uploadDone = state.SnapshotComplete; | ||
| var uploadSymbol = uploadDone ? "[green]●[/]" : "[yellow]○[/]"; | ||
| lines.Add(new Markup($" {uploadSymbol} Uploading [dim]{chunksUploaded:N0} unique chunks[/]{queuePart}")); | ||
| } | ||
| else | ||
| { | ||
| lines.Add(new Markup(" [grey] Uploading[/]")); | ||
| } |
Contributor
There was a problem hiding this comment.
Treat the uploading header as active before the first completion.
This branch stays grey until a chunk has already finished or queue depth is non-zero. The first in-flight upload with queue depth 0, and fully deduped/empty runs where only SnapshotComplete flips, still render as "not started."
💡 Suggested fix
- if (chunksUploaded > 0 || tarsUploaded > 0 || queueDepth > 0)
+ var hasActiveUploads =
+ state.SnapshotComplete ||
+ chunksUploaded > 0 ||
+ tarsUploaded > 0 ||
+ queueDepth > 0 ||
+ state.TrackedFiles.Values.Any(f => f.State == FileState.Uploading) ||
+ state.TrackedTars.Values.Any(t => t.State == TarState.Uploading);
+
+ if (hasActiveUploads)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Arius.Cli/CliBuilder.cs` around lines 893 - 910, The uploading header
should be considered active as soon as an upload is in-flight (even before any
chunk/tar count or queue depth appears) — update the condition that decides the
active vs grey branch to treat the header as active when SnapshotComplete is
false or when ChunksUploaded, TarsUploaded, or UploadQueueDepth indicate work;
specifically, change the if that currently checks (state.ChunksUploaded > 0 ||
state.TarsUploaded > 0 || queueDepth > 0) to also consider
state.SnapshotComplete (e.g., if (!state.SnapshotComplete ||
state.ChunksUploaded > 0 || state.TarsUploaded > 0 || queueDepth > 0)) and
ensure uploadSymbol still uses state.SnapshotComplete to show completed vs
in-progress.
- Show uploading header as active (yellow ○) when scan is complete but snapshot is not yet done, even before first chunk finishes uploading - Replace TrackedTars.Count+1 bundle numbering with a monotonic Interlocked counter (NextBundleNumber) on ProgressState to prevent ID reuse under concurrent TarBundleStartedEvents - Expose TarTargetSize on ProgressState (mirrored from ArchiveOptions) so TarBundleStartedHandler no longer hard-codes 64 MB - Make SetFileUploading return bool to eliminate ContainsKey TOCTOU race in ChunkUploadingHandler; fall through to TAR lookup on false
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Arius.Cli/ProgressHandlers.cs`:
- Around line 88-98: The code can throw InvalidOperationException between Count
check and Keys.Max() due to concurrent modification; wrap the Max/TryGetValue
sequence in a try/catch for InvalidOperationException (or alternatively take an
atomic snapshot under the same lock if you have one) so the handler safely bails
out when the dictionary empties; specifically, around
state.TrackedTars.Keys.Max() and state.TrackedTars.TryGetValue(maxKey, out var
tar) catch InvalidOperationException and return/skip updating tar.AddEntry using
notification.CurrentTarSize and tar.AccumulatedBytes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5aea4a4d-2b0a-4b87-a7b0-ee95b267ceb9
📒 Files selected for processing (3)
src/Arius.Cli/CliBuilder.cssrc/Arius.Cli/ProgressHandlers.cssrc/Arius.Cli/ProgressState.cs
✅ Files skipped from review due to trivial changes (1)
- src/Arius.Cli/CliBuilder.cs
Comment on lines
+88
to
+98
| // Update the current tar (last in the dictionary by bundle number) | ||
| if (state.TrackedTars.Count > 0) | ||
| { | ||
| var maxKey = state.TrackedTars.Keys.Max(); | ||
| if (state.TrackedTars.TryGetValue(maxKey, out var tar)) | ||
| { | ||
| // Derive file size from the current tar size delta | ||
| var addedBytes = notification.CurrentTarSize - (tar.AccumulatedBytes); | ||
| tar.AddEntry(addedBytes > 0 ? addedBytes : 0); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
TOCTOU race: Keys.Max() can throw if dictionary empties between checks.
Between checking Count > 0 and calling Keys.Max(), concurrent removal could leave the collection empty, causing InvalidOperationException. Use DefaultIfEmpty or a single atomic lookup pattern.
🛡️ Proposed fix
// Update the current tar (last in the dictionary by bundle number)
- if (state.TrackedTars.Count > 0)
- {
- var maxKey = state.TrackedTars.Keys.Max();
- if (state.TrackedTars.TryGetValue(maxKey, out var tar))
- {
- // Derive file size from the current tar size delta
- var addedBytes = notification.CurrentTarSize - (tar.AccumulatedBytes);
- tar.AddEntry(addedBytes > 0 ? addedBytes : 0);
- }
- }
+ var maxKey = state.TrackedTars.Keys.DefaultIfEmpty(-1).Max();
+ if (maxKey >= 0 && state.TrackedTars.TryGetValue(maxKey, out var tar))
+ {
+ // Derive file size from the current tar size delta
+ var addedBytes = notification.CurrentTarSize - tar.AccumulatedBytes;
+ tar.AddEntry(addedBytes > 0 ? addedBytes : 0);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Update the current tar (last in the dictionary by bundle number) | |
| if (state.TrackedTars.Count > 0) | |
| { | |
| var maxKey = state.TrackedTars.Keys.Max(); | |
| if (state.TrackedTars.TryGetValue(maxKey, out var tar)) | |
| { | |
| // Derive file size from the current tar size delta | |
| var addedBytes = notification.CurrentTarSize - (tar.AccumulatedBytes); | |
| tar.AddEntry(addedBytes > 0 ? addedBytes : 0); | |
| } | |
| } | |
| // Update the current tar (last in the dictionary by bundle number) | |
| var maxKey = state.TrackedTars.Keys.DefaultIfEmpty(-1).Max(); | |
| if (maxKey >= 0 && state.TrackedTars.TryGetValue(maxKey, out var tar)) | |
| { | |
| // Derive file size from the current tar size delta | |
| var addedBytes = notification.CurrentTarSize - tar.AccumulatedBytes; | |
| tar.AddEntry(addedBytes > 0 ? addedBytes : 0); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Arius.Cli/ProgressHandlers.cs` around lines 88 - 98, The code can throw
InvalidOperationException between Count check and Keys.Max() due to concurrent
modification; wrap the Max/TryGetValue sequence in a try/catch for
InvalidOperationException (or alternatively take an atomic snapshot under the
same lock if you have one) so the handler safely bails out when the dictionary
empties; specifically, around state.TrackedTars.Keys.Max() and
state.TrackedTars.TryGetValue(maxKey, out var tar) catch
InvalidOperationException and return/skip updating tar.AddEntry using
notification.CurrentTarSize and tar.AccumulatedBytes.
This was referenced Mar 28, 2026
6 tasks
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hashing 100% / Queued in TARdisplay dead-ends — files only appear while actively hashing or uploading, then disappearAccumulating → Sealing → Uploading) and byte-level upload progress●(green) on completion[N pending]when the pipeline has backpressure○(yellow) to●(green) when their stage completesChanges
Core (
Arius.Core)FileScannedEventchanged from(long TotalFiles)to(string RelativePath, long FileSize)— per-file event instead of a running totalScanCompleteEvent(long TotalFiles, long TotalBytes)— fires once enumeration finishesTarBundleStartedEvent()— fired by tar builder when a new tar is initializedOnHashQueueReady/OnUploadQueueReadycallbacks toArchiveOptionsfor live queue depth reportingCLI (
Arius.Cli)ProgressState: simplifiedFileStateenum (removedQueuedInTar/UploadingTar, addedHashed); newTrackedTar/TarState; new aggregate fields (FilesScanned,FilesUnique,HashQueueDepth,UploadQueueDepth)BuildArchiveDisplayredesigned with new layout; Spectre bracket escaping fixed ([[N pending]])ProgressTestsfully rewritten — 105 tests, all passingSummary by CodeRabbit
New Features
Documentation
Tests