Skip to content

Count desktop history import progress per session - #1832

Merged
alexeyzimarev merged 2 commits into
mainfrom
capacitor/agent-ee3e0040b69841
Oct 23, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
capacitor/agent-ee3e0040b69841

Conversation

@alexeyzimarev

@alexeyzimarev alexeyzimarev commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Closes #1830 — AI-4218

What & why

The setup wizard's Done page and the main window's import line counted sessions only from kcap import's closing totals line, so a running import read "0 / 245" until a whole pass ended. The app now counts the per-session lines the CLI already prints when its output is redirected, from both the chain worker (Claude, Codex) and the routed loop, and each pass's totals line settles the figures. The progress bar tracks sessions checked against the selection's count, and a spinner shows while the import runs, with "N of M sessions checked" on the Done page.

Where to look

Too-short and excluded sessions print no line, so the bar can stop short of full before the run ends; the totals line corrects the landed and failed counts.

Verification

  • HistoryStepViewModelTests.The_run_counts_sessions_as_they_land: chain-worker and routed lines counted mid-pass, then reconciled by the totals line across two passes
  • DoneStepViewModelTests.A_running_import_shows_sessions_as_they_land: 20 landed, "25 of 100 sessions checked", bar at 25 while running
  • A_pass_without_totals_keeps_its_live_counts, An_uncounted_repository_leaves_the_expected_count_unknown - Desktop app unit suite: 4,177 total, 0 failed

🤖 Generated with Claude Code

kcap import prints one line per settled session when its output is redirected; each pass's
closing totals line still settles the figures, since it also counts sessions that print none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-22T15:12:06.825516Z 6f5678f PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 11 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: d78669ad-74fa-481d-8479-48012de0434c
📥 Commits

Reviewing files that changed from the base of the PR and between 0628b86 and ca09693.

📒 Files selected for processing: 8
  • src/Capacitor.App/ViewModels/MainWindowViewModel.cs
  • src/Capacitor.App/ViewModels/Onboarding/DoneStepViewModel.cs
  • src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs
  • src/Capacitor.App/ViewModels/Onboarding/HistoryStepViewModel.cs
  • src/Capacitor.App/Views/MainWindow.axaml
  • src/Capacitor.App/Views/Onboarding/DonePaneView.axaml
  • test/Capacitor.App.Tests.Unit/DoneStepViewModelTests.cs
  • test/Capacitor.App.Tests.Unit/HistoryStepViewModelTests.cs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Count desktop history import progress per session

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Count sessions as import output arrives instead of waiting for each pass to finish.
• Reconcile uploaded and failed counts with pass totals, including sessions without individual
 output.
• Show sessions checked, progress, and running indicators in the Done page and main window.
Diagram

graph TD
    CLI["kcap import output"] --> Run["History import run"] --> DoneVM["Done view model"] --> DoneView["Done page"]
    Run --> MainVM["Main view model"] --> MainView["Main window"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Structured CLI progress events
  • ➕ Avoids parsing human-readable output.
  • ➕ Could explicitly report sessions that produce no individual line.
  • ➖ Requires a new CLI-to-desktop output contract and changes beyond the desktop app.

Recommendation: Use the PR's existing-output approach for this focused fix: it provides live feedback without changing the CLI, while pass totals remain authoritative for uploaded and failed counts. Consider structured events if the output format becomes unstable or exact checked-session accounting becomes necessary.

Files changed (7) +144 / -15

Enhancement (3) +23 / -7
MainWindowViewModel.csExpose the history import's running state +5/-1

Expose the history import's running state

• Adds a running property and raises its change notification alongside import status so the main window can toggle its activity indicator.

src/Capacitor.App/ViewModels/MainWindowViewModel.cs

MainWindow.axamlShow a spinner beside the main-window import status +8/-3

Show a spinner beside the main-window import status

• Adds an activity icon visible and animated only while the history import is running.

src/Capacitor.App/Views/MainWindow.axaml

DonePaneView.axamlShow import activity above the Done-page progress bar +10/-3

Show import activity above the Done-page progress bar

• Groups a running spinner and checked-session message with the progress bar, visible while the import runs.

src/Capacitor.App/Views/Onboarding/DonePaneView.axaml

Bug fix (2) +55 / -8
DoneStepViewModel.csReport checked sessions and session-based progress +15/-2

Report checked sessions and session-based progress

• Calculates progress from processed sessions against the expected count, falling back to completed passes when no count is available. Adds activity text for the waiting and in-progress states.

src/Capacitor.App/ViewModels/Onboarding/DoneStepViewModel.cs

HistoryImportRun.csCount streamed sessions and reconcile pass totals +40/-6

Count streamed sessions and reconcile pass totals

• Tracks processed, uploaded, and failed sessions from per-session CLI lines. Each pass's closing totals adjust uploaded and failed counts without double-counting the lines already observed.

src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs

Tests (2) +66 / -0
DoneStepViewModelTests.csTest live Done-page figures and progress +37/-0

Test live Done-page figures and progress

• Verifies that streamed sessions update the uploaded count, checked-session message, and progress while the import is still running.

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

HistoryStepViewModelTests.csTest live counts and per-pass reconciliation +29/-0

Test live counts and per-pass reconciliation

• Checks mid-pass counts from session lines and final uploaded, failed, and processed figures across two passes, including an unrelated diagnostic line.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f5678fe79

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@qodo-code-review

qodo-code-review Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Progress bar stops short for many sessions ✓ Resolved
Description
SessionLine() rejects file-chain Loading <id>... <n> lines [new] lines and file-error `Skipping
<id> [reason] lines, while OnLine classifies failures only when a line begins with Failed`. When
those lines appear, their sessions do not increment Processed or the appropriate live outcome
count; the closing totals repair imported and failed counts but not Processed, leaving the Done
page activity text and progress bar below the selection count.
Code

src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[R140-142]

+    // One line per session as it settles, printed by the redirected-output import loop.
+    [GeneratedRegex(@"^(Loading|Skipping|Failed|Already loaded|Refreshed) [^\s()]+(?: \(.*\))?$")]
+    private static partial Regex SessionLine();
Relevance

●●● Strong

Progress counters must include all recognized session outcomes, including formatted failures and
file-chain completions.

PR-#30
PR-#186

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The CLI emits file-chain completion lines with a line-count suffix and file-error lines with a
bracketed reason, but the new regex accepts only an identifier with an optional parenthesized
suffix. Because SessionLine() must match before the live counters change, neither format
increments Processed; the Skipping error also cannot increment Failed under the current
prefix-based classification. The totals handler adjusts Imported and Failed only, so it does not
correct the Processed undercount.

Show per-session desktop history import progress
src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[119-142]
src/Capacitor.Cli/Commands/ImportCommand.cs[1410-1427]
src/Capacitor.Cli/Commands/ImportCommand.cs[1416-1431]
src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[119-126]
src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[140-142]
src/Capacitor.Cli/Commands/ImportCommand.cs[1405-1413]
src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[119-130]

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

## Issue description
The session-line parser excludes file-chain `Loading` completion lines and file-error `Skipping` lines. This omits those sessions from `Processed` and their live outcome counts, and the closing totals do not repair progress.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[119-142]
- src/Capacitor.Cli/Commands/ImportCommand.cs[1405-1431]

## Recommended Fix
Update `SessionLine()` to recognize the file-chain `Loading` line-count format and bracketed-reason `Skipping` format alongside routed completion lines, without accepting diagnostic lines such as `Failed to reach the server`. Count the file-error `Skipping` form as a failure while retaining the routed `Skipping ... (already current)` classification as a skip. Add tests using representative redirected output from both paths and both `Skipping` forms, asserting the live outcome counts, `Processed`, and the progress display.

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



Remediation recommended

2. Two test comments repeat their test names ✗ Dismissed
Description
The added XML comments above A_running_import_shows_sessions_as_they_land and
The_run_counts_sessions_as_they_land merely restate the behavior already expressed by each test
name and assertions. They provide no invariant, constraint, workaround, or rationale, so later
readers must maintain redundant prose when the tests change.
Code

test/Capacitor.App.Tests.Unit/DoneStepViewModelTests.cs[67]

+    /// A run still in flight shows what has landed so far and how far through the selection it is.
Relevance

●●● Strong

Recent history accepts removing test comments that merely restate names and assertions.

PR-#1796
PR-#1775
PR-#1221

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed test files add comments that summarize the behavior under test rather than documenting a
behavior-critical constraint, workaround, invariant, or rationale. This conflicts with the
requirement to restrict comments to non-obvious, behavior-critical constraints.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.App.Tests.Unit/DoneStepViewModelTests.cs[67-70]
test/Capacitor.App.Tests.Unit/HistoryStepViewModelTests.cs[155-157]

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

## Issue description
The new XML comments above the added tests repeat the behavior already communicated by their test names and assertions without documenting a non-obvious constraint or rationale.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/DoneStepViewModelTests.cs[67-67]
- test/Capacitor.App.Tests.Unit/HistoryStepViewModelTests.cs[155-155]

## Recommended Fix
Remove both redundant XML comments, or rewrite them only if they document a specific non-obvious constraint or invariant that is not apparent from the test name and assertions.

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


3. Unknown history yields false progress ✓ Resolved
Description
ProgressValue and ActivityLine treat every positive Expected as a complete selection count,
although StartImport computes it by replacing unknown repository counts with zero. When a
selection contains both a counted and an uncounted repository, the bar uses only the counted subset
as its denominator and the Done page presents that partial number as the total sessions to check.
Code

src/Capacitor.App/ViewModels/Onboarding/DoneStepViewModel.cs[R75-76]

+    public double ProgressValue => Import switch {
+        { Expected: > 0 } run => Math.Min(100.0, 100.0 * run.Processed / run.Expected),
Relevance

●●● Strong

Unknown counts should remain indeterminate, not appear as complete partial totals.

PR-#1729
PR-#186

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The selection UI already treats any missing repository count as an unknown total, but run creation
substitutes zero for each missing count and the new UI branches solely on whether the resulting sum
is positive.

src/Capacitor.App/ViewModels/Onboarding/HistoryRepoRow.cs[49-54]
src/Capacitor.App/ViewModels/Onboarding/HistoryStepViewModel.cs[308-310]
src/Capacitor.App/ViewModels/Onboarding/HistoryStepViewModel.cs[328-334]
src/Capacitor.App/ViewModels/Onboarding/DoneStepViewModel.cs[75-85]

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

## Issue description
The new session bar and activity text present a partial repository count as a complete denominator when any selected repository has an unknown count.
## Fix Focus Areas
- src/Capacitor.App/ViewModels/Onboarding/HistoryStepViewModel.cs[308-310]
- src/Capacitor.App/ViewModels/Onboarding/HistoryStepViewModel.cs[328-334]
- src/Capacitor.App/ViewModels/Onboarding/DoneStepViewModel.cs[75-85]
## Recommended Fix
Represent the run's expected total as unknown if any selected repository lacks a count, and use the pass-based fallback and denominator-free activity text for those runs.

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


4. A failed pass erases earlier upload counts ✓ Resolved
Description
OnLine resets _passImported and _passFailed only when it sees a totals line, but RunAsync
continues to the next pass after a nonzero or timed-out result. If the first pass emits session
lines and exits without totals, the next pass subtracts those stale counters from its own totals,
reducing the run’s previously recorded Imported or Failed count.
Code

src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[R112-115]

+            Imported      = Add(Imported, imported - _passImported);
+            Failed        = Add(Failed, failed - _passFailed);
+            _passImported = 0;
+            _passFailed   = 0;
Relevance

●●● Strong

Concrete retry-state accounting bug conflicts with the PR’s multi-pass reconciliation intent.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The run continues after a failed result, while only totals reset the new per-pass counters; the
process runner can return a timeout after delivering partial output.

src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[87-94]
src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[109-130]
src/Capacitor.Cli.Core/ProcessRunner.cs[204-211]

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

## Issue description
A pass without closing totals leaves live per-pass counters in place, so the following pass's totals subtract counts belonging to the earlier pass.
## Fix Focus Areas
- src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[87-94]
- src/Capacitor.App/ViewModels/Onboarding/HistoryImportRun.cs[109-130]
## Recommended Fix
Clear per-pass reconciliation counters at every pass boundary, including missing-totals failures, with ordering that preserves already-posted line callbacks. Test a partial-output failed first pass followed by a successful second pass.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 59 rules
✅ Cross-repo context — repo relationships
Review mode: Auto: ⚖️ Balanced: Behavioral progress parsing spans runtime state, UI bindings, and multiple code paths.

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Claude and Codex history reports through the chain worker's own line forms, so without them the
figures stayed at zero for the common case. A selection with an uncounted repository now has no total.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop history import shows no progress until a pass ends

1 participant