Fix per-notifier error/warning count drift in dispatch chain

[paolostivanin]

Snapshot ErrorCount, WarningCount and LogCategories into BackupStats once at backup completion (finalizeBackupStats / parseFailedBackupLogCounts) and have convertBackupStatsToNotificationData read them as-is. Re-parsing the log file per-notifier was over-counting warnings emitted by earlier notifiers in the dispatch chain, so e.g. Pushover reported 5 warnings while Email reported 0 on the same run.

Summary by CodeRabbit

• Bug Fixes & Improvements

  • Notifications now capture consistent pre-send snapshots of backup stats (error/warning counts and log categories), preventing timing-related mismatches.
  • Backup reporting includes categorized log information and no longer re-reads log files during notification dispatch.
  • Dry-run behavior now centralizes how issue counts affect exit codes.

• Tests

  • Added behavioral tests to verify snapshot timing, warning propagation, and early-error log preservation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c30ba678-3c6f-4290-9975-256ec350ab9d

📥 Commits

Reviewing files that changed from the base of the PR and between 49737f0 and 3038fc1.

📒 Files selected for processing (4)

• internal/orchestrator/additional_helpers_test.go
• internal/orchestrator/backup_run_phases.go
• internal/orchestrator/extensions.go
• internal/orchestrator/extensions_additional_test.go

---

📝 Walkthrough

Walkthrough

This PR refactors the backup orchestrator's notification dispatch to introduce a snapshot boundary that captures pre-notification state before issuing channel notifications, centralizes log-count parsing through a shared refresh helper, and simplifies notification conversion to trust precomputed snapshots rather than re-reading logs.

Changes

Pre-Notification Issue Snapshot Boundary

Layer / File(s)	Summary
BackupStats Schema Extension and Import internal/orchestrator/orchestrator.go	BackupStats struct now carries LogCategories []notify.LogCategory field alongside existing error/warning counts, and imports the notify package.
Notification Snapshot Boundary Implementation internal/orchestrator/extensions.go	New startNotificationGroup() wrapper and snapshotPreNotificationIssues() helper refresh BackupStats from log files (populating error/warning counts and optionally log categories) before dispatching notifications.
Integrate Boundary into Dispatch Call Sites internal/orchestrator/extensions.go	DispatchEarlyErrorNotification and dispatchNotificationsAndLogs now route through the new notification boundary, ensuring pre-notification snapshots occur in both success and error paths.
Centralize Log Parsing via Snapshot Refresh internal/orchestrator/backup_run_helpers.go, internal/orchestrator/backup_run_phases.go	parseFailedBackupLogCounts and finalizeBackupStats now call refreshLogIssuesFromFile() instead of directly invoking ParseLogCounts(), consolidating log-count derivation and modifying dry-run behavior.
Simplify Notification Conversion to Use Snapshots internal/orchestrator/notification_adapter.go	convertBackupStatsToNotificationData now consumes precomputed error/warning counts and log categories from BackupStats snapshots via defensive copy, removing prior log re-parsing fallback logic.
Test Stubs and Behavioral Test for Snapshot Timing internal/orchestrator/additional_helpers_test.go	Extended stubNotifierChannel to record snapshot values, introduced warningStorageTarget stub, and added TestDispatchPostBackupSnapshotsIssuesImmediatelyBeforeNotifications to validate snapshot capture and warning preservation across the boundary.
Adapter Conversion Test Updates internal/orchestrator/notification_adapter_test.go	Replaced and added tests to verify convertBackupStatsToNotificationData propagates snapshot data without re-reading logs, validates defensive LogCategories copying, and confirms zero counts are preserved when logs are not re-parsed.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 A snapshot's taken, swift and true,
Before the notifiers get their cue,
No re-reads in the flow—one state held fast,
Counts and categories bound at last! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title directly and clearly summarizes the main change: fixing a bug where error/warning counts drift across notifiers in the dispatch chain by snapshotting counts at backup completion rather than re-parsing per notifier.
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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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.}

[codacy-production]

Not up to standards ⛔

🔴 Issues 2 medium

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category	Results
Complexity	2 medium

View in Codacy

🟢 Metrics 58 complexity · 5 duplication

Metric	Results
Complexity	58
Duplication	5

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[codecov]

Codecov Report

❌ Patch coverage is 78.72340% with 10 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/orchestrator/extensions.go	81.25%	4 Missing and 2 partials ⚠️
internal/orchestrator/backup_run_phases.go	63.63%	3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[tis24dev]

@paolostivanin I made a change and moved the snapshot to a later time.
Let me know what you think.

[tis24dev]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@internal/orchestrator/extensions.go`:
- Around line 129-145: In refreshLogIssuesFromFile, ParseLogCounts currently
unconditionally overwrites BackupStats.ErrorCount and WarningCount; change this
so parsed counts only replace the existing stats if they are > 0 (or if the
existing value is 0 and parsed >0), e.g. use the returned
errorCount/warningCount to update stats.ErrorCount and stats.WarningCount only
when those parsed values are positive (preserving pre-seeded counts such as
ErrorCount=1); keep the existing behavior for LogCategories controlled by
includeCategories and the ParseLogCounts call.

🪄 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: 74144e62-32ac-4364-9d10-b3c7b4d475f5

📥 Commits

Reviewing files that changed from the base of the PR and between 467cd96 and 49737f0.

📒 Files selected for processing (7)

• internal/orchestrator/additional_helpers_test.go
• internal/orchestrator/backup_run_helpers.go
• internal/orchestrator/backup_run_phases.go
• internal/orchestrator/extensions.go
• internal/orchestrator/notification_adapter.go
• internal/orchestrator/notification_adapter_test.go
• internal/orchestrator/orchestrator.go

[tis24dev]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[paolostivanin]

sorry I could answer only now! Thanks again for the fast feedback and merge 😄