Performance bucket from audit (#531): O(n²) model build, SSE rebuild thrash, log buffer

[nitrobass24]

Fixes the ⚡ Performance bucket of tracking issue #531 (3 issues) — behavior-preserving optimizations with tests.

Note

Stacked on #538 (frontend bucket): #521/#522 touch the logs/file-list code #538 also changed, so this branch builds on it. GitHub will auto-retarget this PR to develop once #538 merges, at which point the diff shows only these performance commits. Intentional multi-issue grouping (per the maintainer's request), one commit per issue.

How this was built

A multi-agent workflow: parallel investigation → sequential implementation (one tested commit per issue) → parallel adversarial review, with the review explicitly checking each optimization is strictly behavior-preserving (identical output, only faster), not just that tests pass.

Issues fixed

#	Stack	Fix
#520	Python	build_model() (every 0.5s on the controller thread) used list.pop(0) in two per-file BFS traversals → O(n²) on large trees, plus a copy.copy of each node's child list. Now collections.deque + popleft() (O(1)) and a non-copying iter_children() on the hot read-only path. Byte-identical model output; big asymptotic win on deep/wide seedbox trees.
#521	Angular	ViewFileService rebuilt the whole file list on every SSE update. Now coalesces SSE-driven rebuilds, memoizes the filter, and preserves ViewFile object identity for unchanged rows so OnPush/trackBy can skip them; single-pass bulk remove. Same rendered view (files, order, checked/selected state).
#522	Angular	Logs page kept an unbounded live buffer and re-rendered per record. Now a ring-buffer cap (newest kept, oldest front-trimmed) + a stable, collision-free trackBy.

Review findings → resolution

• Performance: backend model-build is O(n²); related per-cycle hot-path costs #520, Performance: frontend file-list rebuild thrash on SSE updates #521: passed clean — behaviorPreserved=true, all acceptance criteria met.
• Performance: Logs page unbounded buffer + full re-render per record #522 (was blocking): the agent's first trackBy keyed by time+logger+message, which collides for identical repeated log lines in the same millisecond (common under DEBUG sync) → Angular NG0955 duplicate-key reconcile drops/mis-associates rows. Fixed with a per-object monotonic seq key (WeakMap) — unique even for identical content, stable per object. Added a duplicate-identical-lines render guard.
• Performance: Logs page unbounded buffer + full re-render per record #522 remaining ACs deferred → Logs page: CDK virtual scroll + batched change detection (follow-up to #522) #539: CDK virtual scroll and batched change detection. Virtual scroll doesn't cleanly fit the logs page's variable-height rows (wrapped messages + inline tracebacks) and the current window-scroll model; batching needs fake-timer tests. The ring-buffer cap (the primary unbounded-memory hazard) + collision-free trackBy ship here; the virtualization/batching enhancements are tracked in Logs page: CDK virtual scroll + batched change detection (follow-up to #522) #539.

Test plan

• ✅ Full Angular suite (Vitest): 473 passed (37 files); ng lint --max-warnings 0 clean.
• ✅ Full Python unit suite: 849 passed (the lone failure, test_scan_file_with_latin_chars, is a pre-existing macOS-only Errno 92 that passes on CI's Linux runner); ruff check + C901 (complexity ≤ 12) clean.
• New tests: deeper/larger-tree BFS correctness (Performance: backend model-build is O(n²); related per-cycle hot-path costs #520), view-rebuild coalescing + identity preservation (Performance: frontend file-list rebuild thrash on SSE updates #521), buffer cap + duplicate-line trackBy guard (Performance: Logs page unbounded buffer + full re-render per record #522).

Closes #520. Closes #521. Closes #522.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added two-click confirmation for delete actions to prevent accidental deletions.

• Bug Fixes

  • File action errors now display notifications.
  • Log history load failures now show error messages.
  • Improved connection retry logic with exponential backoff for better stability.
  • Enhanced error recovery for malformed server data.

• Style

  • Added visual confirmation state styling for delete buttons.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3d616bd8-64f6-42c0-9a47-c90e99d976f4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR introduces structured error-handling via file-action event payloads, defensive JSON parsing across SSE services, view-file emission coalescing with object-identity optimization, logs-page buffer management and stable tracking, exponential-backoff stream reconnection, credential-redaction guards in config/interceptor, improved settings toggle error recovery, and a Python iterator API for efficient model traversal.

Changes

Angular Frontend Resilience & Performance

Layer / File(s)	Summary
File action event system and error handling src/angular/src/app/pages/files/file.component.ts, src/angular/src/app/pages/files/file-list.component.ts, src/angular/src/app/pages/files/bulk-action-bar.component.ts, src/angular/src/app/pages/files/*.spec.ts	FileComponent exports FileActionEvent interface and updates all action outputs to emit payloads with file and clearActiveAction callback. FileListComponent refactors handlers to accept FileActionEvent, centralizes success/failure handling with notifications and child-action clearing. BulkActionBarComponent implements double-click confirmation flow with 3-second auto-reset and lifecycle cleanup.
View file emission coalescing and optimization src/angular/src/app/services/files/view-file.service.ts, src/angular/src/app/app.config.ts, src/angular/src/app/services/files/view-file.service.spec.ts	ViewFileService exports VIEW_FILE_COALESCE_MS injection token for configurable SSE buffering via auditTime; conditionally replaces only rows whose isChecked/pairName actually changes; single-pass removal with Set; filtered-array memoization to skip re-emits when membership is reference-identical. App config wires provider at 100ms. Tests verify coalescing window, identity preservation, and memoization.
SSE/JSON parsing defensive guards src/angular/src/app/services/files/model-file.service.ts, src/angular/src/app/services/logs/log.service.ts, src/angular/src/app/services/server/server-status.service.ts, src/angular/src/app/services/autoqueue/autoqueue.service.ts, src/angular/src/app/services/*/\*.spec.ts	ModelFileService, LogService, ServerStatusService, and AutoQueueService add nested try/catch guards around JSON.parse and shape validation; log errors via injected LoggerService; skip/skip-and-recover from malformed events without propagating exceptions. Comprehensive tests cover malformed JSON, wrong shapes, and valid-event recovery.
Logs page buffer capping and stable tracking src/angular/src/app/pages/logs/logs-page.component.ts, src/angular/src/app/pages/logs/logs-page.component.html, src/angular/src/app/pages/logs/logs-page.component.scss, src/angular/src/app/pages/logs/logs-page.component.spec.ts	LogsPageComponent adds MAX_LIVE_RECORDS=2000 to cap live buffer with front-trimming; historyLoadFailed state for error vs empty distinction; stable trackRecord/trackHistory functions backed by WeakMap monotonic IDs prevent key collisions. Template renders failure UI and uses custom tracking. History fetch wrapped in switchMap/catchError to handle errors and return empty on failure.
Stream reconnection exponential backoff src/angular/src/app/services/base/stream-dispatch.service.ts, src/angular/src/app/services/base/stream-dispatch.service.spec.ts	StreamDispatchService replaces fixed RETRY_INTERVAL_MS with capped exponential backoff: RETRY_BASE_MS, RETRY_MAX_MS, RETRY_JITTER_MS constants and retryAttempt counter reset on key change and successful open. nextRetryDelayMs() computes delay with exponential growth, capping, and jitter. Tests validate timing, exponential growth, reset, and jitter via fake timers.
Config service credential redaction handling src/angular/src/app/services/settings/config.service.ts, src/angular/src/app/services/utils/api-key.interceptor.ts, src/angular/src/app/services/settings/config.service.spec.ts, src/angular/src/app/services/utils/api-key.interceptor.spec.ts	ConfigService imports REDACTED_SENTINEL; uses preserveRealApiKey() to prevent sentinel overwriting session real key; fetches config immediately in constructor; syncs stream API key directly on web.api_key changes; syncStreamApiKey early-returns on sentinel. ApiKeyInterceptor suppresses X-Api-Key header when sentinel is detected. Tests cover init ordering, sentinel non-propagation, and real-key preservation.
Settings toggle error handling and recovery src/angular/src/app/pages/settings/path-pairs.component.ts, src/angular/src/app/pages/settings/path-pairs.component.spec.ts	PathPairsComponent centralizes toggle behavior via applyToggle() helper: clears prior error, calls update, sets error message and calls refresh() on falsy/error result, triggers markForCheck. Tests verify error clearing on success and error+refresh behavior on failure.

Python Backend Iterator API

Layer / File(s)	Summary
Model file iterator API and BFS optimization src/python/model/file.py, src/python/controller/model_builder.py, src/python/tests/unittests/test_model/test_file.py, src/python/tests/unittests/test_controller/test_model_builder.py	ModelFile adds iter_children() method returning read-only iterator over live child list. ModelBuilder switches traversal frontiers from list.pop(0) to collections.deque.popleft() for O(1) dequeue; _build_children and _check_root_downloaded use deque and new iterator to avoid list copying. Stress tests added using deep-wide tree builder verifying DOWNLOADED/DEFAULT state transitions.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• nitrobass24/seedsync#522: LogsPageComponent enhancements (MAX_LIVE_RECORDS, stable trackBy, error state) directly address this issue's live-log buffer capping and high-throughput duplicate-line handling.
• nitrobass24/seedsync#520: ModelBuilder BFS optimization (deque + iter_children) directly implements the recommended fix for O(n²) traversal via list.pop(0)/get_children() copying.
• nitrobass24/seedsync#521: ViewFileService coalescing, object-identity preservation, single-pass removal, and filtered-array memoization directly address the same code-level view-rebuild thrashing issues.
• nitrobass24/seedsync#531: Changes implement multiple frontend follow-ups tracked in the audit (file-action failure surfacing, bulk-delete confirmation, SSE JSON-parse guards, view-file coalescing, logs buffer cap).
• nitrobass24/seedsync#539: LogsPageComponent updates add MAX_LIVE_RECORDS buffer cap and stable tracking, implementing part of the requested follow-up (though not CDK virtual-scroll or batched change detection).

Possibly related PRs

• nitrobass24/seedsync#245: PR modifies the same Angular services (StreamDispatchService reconnection, ConfigService API-key/sentinel handling) whose new unit-test coverage is added in the main PR.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly describes the main performance fixes: O(n²) model build, SSE rebuild thrash, and log buffer issues that are addressed in this PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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 perf/performance-bucket-531

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/angular/src/app/pages/logs/logs-page.component.ts (1)

89-99: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Set pendingScrollToBottom before detectChanges() so live tail auto-scroll isn’t missed

src/angular/src/app/pages/logs/logs-page.component.ts calls this.changeDetector.detectChanges() before this.pendingScrollToBottom = true. Since ngAfterViewChecked() only scrolls when pendingScrollToBottom is already true, the “tail visible” case can miss scrolling for that record update.

🔧 Proposed fix

         this.records = next.length > LogsPageComponent.MAX_LIVE_RECORDS
           ? next.slice(next.length - LogsPageComponent.MAX_LIVE_RECORDS)
           : next;
-        this.changeDetector.detectChanges();
-
         if (shouldScroll) {
           this.pendingScrollToBottom = true;
         }
+        this.changeDetector.detectChanges();
         this.refreshScrollButtonVisibility();

Add a regression spec in src/angular/src/app/pages/logs/logs-page.component.spec.ts (under the existing #522 live-log describe) that:

• stubs component.logTail.nativeElement.getBoundingClientRect() to be inside the viewport
• forces the host offsetParent to be non-null
• spies on window.scrollTo
• emits one live record via the existing logs$ subject
• asserts scrollTo is called as a result of that emission (no extra manual fixture.detectChanges() needed).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/angular/src/app/pages/logs/logs-page.component.ts` around lines 89 - 99,
Move the assignment to pendingScrollToBottom so it happens before
changeDetector.detectChanges(): when appending the new record in
LogsPageComponent set this.pendingScrollToBottom = true (if shouldScroll) and
only then call this.changeDetector.detectChanges(), ensuring
ngAfterViewChecked() will see the flag and trigger scrolling; also add a
regression spec in the LogsPageComponent test: stub
component.logTail.nativeElement.getBoundingClientRect() to return an in-viewport
rect, ensure the host element reports a non-null offsetParent, spy on
window.scrollTo, emit one live record on the existing logs$ subject, and assert
window.scrollTo was called.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/angular/src/app/pages/files/file.component.ts`:
- Around line 150-152: actionEvent currently returns a clearActiveAction
callback that closes over the component instance and can clear a different row
when the component is recycled under *cdkVirtualFor; fix by making the callback
verify the original file identity before mutating activeAction. Modify
actionEvent(FileComponent::actionEvent) to capture a stable identifier from the
passed ViewFile (e.g. file.id or the file object reference) and return
clearActiveAction that checks this.activeAction?.fileId === capturedId (or
this.activeAction?.file === capturedFile) before calling
this.clearActiveAction() or clearing activeAction; update clearActiveAction if
needed to accept/compare an id instead of unconditionally clearing. Ensure
references to FileActionEvent, ViewFile, actionEvent and activeAction are used
so the check is performed on the original file identity.

In `@src/angular/src/app/services/files/view-file.service.ts`:
- Around line 70-85: When rebuilding pairName on existing view files, also
reapply the current sort so ordering stays consistent: in the branch where you
compute nextFiles and detect changed, sort nextFiles with the component's sort
comparator (e.g. call nextFiles.sort(this.sortComparator) or use the existing
sort routine) before assigning this.files = nextFiles and calling
this.pushViewFiles(); this ensures updates to pairName immediately reflect any
sort-by-pairName ordering.

In `@src/angular/src/app/services/settings/config.service.ts`:
- Around line 133-139: The current check uses a truthy test on currentKey so
cleared values ('' or null) are treated as absent and get overwritten by
REDACTED_SENTINEL; change the condition to detect presence of an api_key field
instead of truthiness. In config.service.ts, where currentKey is retrieved from
this.configSubject.getValue()?.web?.api_key and compared against
REDACTED_SENTINEL, replace the truthy check (currentKey && ...) with an
existence check (e.g.,
Object.prototype.hasOwnProperty.call(this.configSubject.getValue()?.web ?? {},
'api_key') or checking for !== undefined) so that empty-string or null stored
keys are preserved when incoming.web?.api_key === REDACTED_SENTINEL.

---

Outside diff comments:
In `@src/angular/src/app/pages/logs/logs-page.component.ts`:
- Around line 89-99: Move the assignment to pendingScrollToBottom so it happens
before changeDetector.detectChanges(): when appending the new record in
LogsPageComponent set this.pendingScrollToBottom = true (if shouldScroll) and
only then call this.changeDetector.detectChanges(), ensuring
ngAfterViewChecked() will see the flag and trigger scrolling; also add a
regression spec in the LogsPageComponent test: stub
component.logTail.nativeElement.getBoundingClientRect() to return an in-viewport
rect, ensure the host element reports a non-null offsetParent, spy on
window.scrollTo, emit one live record on the existing logs$ subject, and assert
window.scrollTo was called.

🪄 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: ASSERTIVE

Plan: Pro

Run ID: b35bae78-b10d-45dd-84d8-cddea87c0d9b

📥 Commits

Reviewing files that changed from the base of the PR and between 416047c and 80d428d.

📒 Files selected for processing (33)

• src/angular/src/app/app.config.ts
• src/angular/src/app/pages/files/bulk-action-bar.component.spec.ts
• src/angular/src/app/pages/files/bulk-action-bar.component.ts
• src/angular/src/app/pages/files/file-list.component.spec.ts
• src/angular/src/app/pages/files/file-list.component.ts
• src/angular/src/app/pages/files/file.component.spec.ts
• src/angular/src/app/pages/files/file.component.ts
• src/angular/src/app/pages/logs/logs-page.component.html
• src/angular/src/app/pages/logs/logs-page.component.scss
• src/angular/src/app/pages/logs/logs-page.component.spec.ts
• src/angular/src/app/pages/logs/logs-page.component.ts
• src/angular/src/app/pages/settings/path-pairs.component.spec.ts
• src/angular/src/app/pages/settings/path-pairs.component.ts
• src/angular/src/app/services/autoqueue/autoqueue.service.spec.ts
• src/angular/src/app/services/autoqueue/autoqueue.service.ts
• src/angular/src/app/services/base/stream-dispatch.service.spec.ts
• src/angular/src/app/services/base/stream-dispatch.service.ts
• src/angular/src/app/services/files/model-file.service.spec.ts
• src/angular/src/app/services/files/model-file.service.ts
• src/angular/src/app/services/files/view-file.service.spec.ts
• src/angular/src/app/services/files/view-file.service.ts
• src/angular/src/app/services/logs/log.service.spec.ts
• src/angular/src/app/services/logs/log.service.ts
• src/angular/src/app/services/server/server-status.service.spec.ts
• src/angular/src/app/services/server/server-status.service.ts
• src/angular/src/app/services/settings/config.service.spec.ts
• src/angular/src/app/services/settings/config.service.ts
• src/angular/src/app/services/utils/api-key.interceptor.spec.ts
• src/angular/src/app/services/utils/api-key.interceptor.ts
• src/python/controller/model_builder.py
• src/python/model/file.py
• src/python/tests/unittests/test_controller/test_model_builder.py
• src/python/tests/unittests/test_model/test_file.py