chore(sonar): cut S107 too-many-parameters via cohesive parameter objects

[thomasluizon]

What

Behavior-preserving burn-down of SonarCloud rule csharpsquid:S107 ("Methods should not have too many parameters", default max 7) in orbit-api. Drives the rule from 28 → 3 open instances.

How

DI-injected constructors (15) — grouped cohesive dependencies into aggregate records, following the existing *Repositories convention already in the codebase (e.g. SendCheerRepositories, AccountabilityRepositories), and registered them in DI:
GoalRepositories, BulkCreateHabitsRepositories, SkipHabitRepositories, GetCalendarEventsRepositories, ApplyOnboardingRepositories, CreateChallengeRepositories, GetFriendProfileRepositories, UserStreakRepositories, ExportUserDataRepositories (18→1), plus SocialInteractionServices (shared by SendCheer + CheckIn), GamificationNotifiers, AgentPendingStores, StripeServiceClients.

Methods / domain factory / interface (10) — introduced cohesive parameter-object records, destructured at method entry to keep bodies unchanged:
AiUsageTotals, DailySummaryContext (ISummaryService + AiSummaryService), StreakBridgeContext, PerUserStreakLookups, AchievementAccumulator, HabitScheduleWindow, AgentSnapshotInputs, ClarificationToolResult, ResolveAuditOutcome.

All call sites and unit-test constructions updated. Records are S107-exempt (positional record constructors are not flagged), so aggregate/parameter records stay clean.

Verification

• dotnet build Orbit.slnx — 0 warnings/errors for S107 targets.
• dotnet test Orbit.slnx — 5284 passed, 0 failed. No behavior change.

Remaining (3, not fixed)

• DistributedRateLimitAttribute.LogRateLimitRejected — a [LoggerMessage] source-generated method whose parameters are the required structured-log fields; folding them into a parameter object would change the emitted log structure. Verified S107 false positive.
• AgentOperationExecutor.CreateAuditContext and IAiIntentService.SendWithToolsAsync — deferred (the latter has a wide test-mock surface); safe to address in a follow-up.

Refs thomasluizon/orbit-ui-mobile#243

[claude]

See full review below.

[claude]

Code Review: PR #391 (thomasluizon/orbit-api)

Recommendation: APPROVE

Summary

Behavior-preserving mechanical refactor grouping DI-injected dependencies and long parameter lists into cohesive parameter-object records to burn down SonarCloud S107 (28 to 3 open instances). Walked all 49 changed files, cross-checked every new aggregate record's field order against its original constructor/parameter order and every call site (incl. destructuring assignments) for argument-order drift, and verified DI lifetimes for the four newly-registered aggregates. No functional or security regressions. One non-blocking documentation defect recurs in two files.

Findings

Critical: None.
High: None.

[MEDIUM] Pre-existing class XML-doc comment orphaned by inserted parameter-object record

• location: src/Orbit.Application/Profile/Commands/ApplyOnboardingCommand.cs (around line 53, where the new ApplyOnboardingRepositories record is inserted), src/Orbit.Infrastructure/Services/StripeBillingService.cs (around line 9, where the new StripeServiceClients record is inserted)
• issue: In both files the PR inserts a new record (ApplyOnboardingRepositories, StripeServiceClients) with its own /// summary directly between the pre-existing class-level /// summary and the class declaration it documents. The two doc blocks are now contiguous with no blank line, so the closest one attaches to the new record and the original rich class doc (idempotency semantics for ApplyOnboardingCommandHandler; SDK-isolation rationale for StripeBillingService) is orphaned.
• risk: Non-blocking (no build/analyzer breakage, GenerateDocumentationFile isn't enabled repo-wide), but a real avoidable loss of the most important doc comment in each file from IDE tooltips / generated docs.
• fix: Move the pre-existing class doc back to sit immediately above its class; place each new record's own summary directly above the record with a blank line separating the two doc'd declarations.

Low/Info: None posted (signal gate drops nits - e.g. ExportUserDataRepositories's eighteen-field aggregate is an intentional, PR-body-acknowledged consolidation).

Subagents

• security-reviewer: PASS. Verified (1) all four newly-DI-registered aggregates (SocialInteractionServices/GamificationNotifiers/AgentPendingStores = Scoped, StripeServiceClients = Singleton) have lifetimes consistent with every constituent service they wrap, no captive-dependency risk; (2) AiController's new ResolveAuditOutcome record preserves the exact same PolicyDecision/Status/Error/TargetName values at both call sites as the pre-refactor positional arguments, no audit-trail regression; (3) Stripe API key still set once at startup, unaffected by the StripeServiceClients grouping.
• contract-aligner: N/A, diff touches no DTO, Controller route, or packages/shared type; purely internal constructor/DI wiring.

Validation

Build/Tests: not independently re-run in this review session (sandbox could not obtain approval for dotnet/git-fetch commands); relying on the PR body's stated CI results (dotnet build, 0 warnings/errors for S107 targets; dotnet test, 5284 passed, 0 failed) plus full static verification of every changed file and call site instead.

Deferred

Dimensions 8 (DESIGN.md/AI-slop), 9 (web/mobile parity), 10 (i18n), 14 (FEATURES.md parity) are N/A, backend-only internal refactor, no user-facing surface changed. Cross-model /second-opinion not invoked, no Critical finding survived to trigger it.

What's good

Follows the repo's existing *Repositories aggregate-record convention; every new record's field order exactly matches the original parameter order at every call site across 49 files with zero drift; DI registrations for the four new container-managed aggregates are lifetime-consistent; PR body is transparent about the three S107 instances intentionally left unfixed and why.

Recommendation

Approve. Fix the two orphaned doc-comment spots (Medium, non-blocking) in a fast-follow or trivial amendment, reordering a few lines in each of the two files.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

PR Review: #391 — chore(sonar): cut S107 too-many-parameters via cohesive parameter objects

Recommendation: APPROVE

Severity	Count
Critical (incl. ⚠️ old-client breaks)	0
High	0
Medium	2
Low / Info	0 (not posted per signal gate)

Findings

[MEDIUM] Orphaned/duplicated XML doc comment — src/Orbit.Application/Profile/Commands/ApplyOnboardingCommand.cs:53-66
The pre-existing class doc for ApplyOnboardingCommandHandler ("Applies the buffer of answers... Idempotent by construction...") now sits directly above the newly-inserted ApplyOnboardingRepositories record, stacked with that record's own /// <summary> into one malformed double-<summary> doc block. ApplyOnboardingCommandHandler itself is left with no doc comment. Fix: move the original block back above the handler class.

[MEDIUM] Same pattern — src/Orbit.Infrastructure/Services/StripeBillingService.cs:9-25
The original StripeBillingService class doc ("Stripe-backed implementation of IBillingService...") is now misattached above the new StripeServiceClients record instead of the service class. Fix: move it back above StripeBillingService.

Verification performed

• Read the full diff (49 files) and cross-checked every new parameter-object record's field order against every construction site and destructuring site (ExportUserDataRepositories (18 fields), DailySummaryContext, AchievementAccumulator, HabitScheduleWindow, StreakBridgeContext, PerUserStreakLookups, AgentSnapshotInputs, ClarificationToolResult, ResolveAuditOutcome, AiUsageTotals, and all DI factory lambdas in ServiceCollectionExtensions.cs) — all correct, consistent with the PR's claimed 5284/5284 passing tests.
• Ran the security-reviewer subagent (gate: any src/ change): PASS — [Authorize] on AiController untouched; Stripe API key still set once at startup; StripeServiceClients (Singleton) depends only on Singleton Stripe SDK clients (no captive-dependency issue); no nullability/validation weakened; no new record leaks into logs or DTOs; all bare AddScoped<T>()/AddSingleton<T>() registrations have compatible dependency lifetimes.
• contract-aligner: N/A — no DTO, Controller route, or packages/shared type touched; all new records are internal DI/parameter aggregates.
• Backward-compat guard: N/A — nothing shipped to mobile clients changed.
• Backend hard rules (timezone, authz, validation, logging, transaction teardown): reviewed, no violations — this diff touches no business logic, only constructor/parameter signatures.
• Deferred: DESIGN.md/AI-slop, Parity, i18n, FEATURES.md parity — N/A, no apps/* or user-facing feature surface touched.
• Validation phase (dotnet build/test): not run locally — CI already gated it (Build / Unit Tests / SonarCloud required checks); PR description states local dotnet build/dotnet test (5284 passed, 0 failed).

Both findings are cosmetic documentation-placement issues introduced by the mechanical refactor, with no runtime, security, or contract impact — worth a quick follow-up but not blocking.

Note: PR was already merged at review time (mergedAt 2026-07-13T22:10:24Z); this review is posted for the record per the automated review workflow.