build: adopt central package management (Version.props + Directory.Packages.props)#2
Merged
Merged
Conversation
…ckages.props) Lifts every PackageReference Version= out of the four production/test csproj files and into root-level Version.props variables consumed by Directory.Packages.props. Mirrors the TourPlanner reference layout so a single file owns the .NET 10 / EF Core 10 / Testcontainers / MTP version surface. - Pipeline opts out via Pipeline/Directory.Packages.props so the NUKE bootstrap stays self-contained (matches the existing Pipeline/Directory.Build.props parent-isolation pattern). - MartinCostello.Logging.XUnit.v3 unified to 0.7.1 (patch bump; was split 0.7.0 vs 0.7.1 across the two test projects — CPM requires a single version). - Verified with ./build.sh Compile — Restore + Compile both succeed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.props⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (6)
Summary by CodeRabbitVeröffentlichungshinweise
WalkthroughZwei neue Dateien führen zentrale NuGet-Paketversionsverwaltung ein: ChangesZentrale Paketversionsverwaltung
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces NuGet Central Package Management (CPM) for the .NET solution by centralizing package version definitions into a shared Version.props consumed by Directory.Packages.props, and updates the existing projects to remove per-project Version= attributes. It also ensures the NUKE build subtree remains self-contained by opting the Pipeline/ folder out of CPM.
Changes:
- Added root-level CPM configuration via
Directory.Packages.propsand a sharedVersion.propscontaining all package version variables. - Updated backend production/test
.csprojfiles to rely on centrally defined versions (removing inlineVersion=pins). - Added
Pipeline/Directory.Packages.propsto disable CPM within the build pipeline subtree.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Version.props | New centralized property definitions for all package versions. |
| Directory.Packages.props | Enables CPM and maps packages to version properties from Version.props. |
| Pipeline/Directory.Packages.props | Disables CPM for the NUKE pipeline subtree to keep bootstrap pinning local. |
| PaperlessServices/PaperlessServices.csproj | Removes inline package versions to use CPM. |
| PaperlessServices.Tests/PaperlessServices.Tests.csproj | Removes inline package versions to use CPM (incl. test/coverage stack). |
| PaperlessREST/PaperlessREST.csproj | Removes inline package versions to use CPM (incl. EF Core, Hangfire, health checks). |
| PaperlessREST.Tests/PaperlessREST.Tests.csproj | Removes inline package versions to use CPM and aligns to centralized test dependency versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3
to
+6
| <Import Project="Version.props" /> | ||
|
|
||
| <PropertyGroup> | ||
| <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally> |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
6 tasks
ANcpLua
added a commit
that referenced
this pull request
May 15, 2026
…k (L20-22)
Drives PaperlessREST/Features/EventProcessing/Presentation/GenAiResultListener.cs
from 58/61 (95.1%) to 60/61 (98.4%) on the CI-aligned dotcov metric. The state
machine <ExecuteAsync>d__5 moves from line-rate 88% / branch-rate 50% to
line-rate 96% / branch-rate 100%.
Adds one fact + one helper iterator to ListenerLifecycleTests.cs:
GenAi_ExecuteAsync_TokenCancelledBetweenYields_BodyBreakCheckFires hits the
`if (stoppingToken.IsCancellationRequested) { break; }` block inside the
await-foreach. The pre-existing StoppingTokenCancelled test cannot reach it —
cancellation through the [EnumeratorCancellation] token short-circuits the
iterator before the loop body re-enters, so the body's IsCancellationRequested
check never fires. The new YieldAfterCancel<T> iterator deliberately ignores
[EnumeratorCancellation], yields the second event after the test cancels the
CTS, and forces the body-internal check to trip + break. Asserts:
- DocumentService.UpdateDocumentSummaryAsync for event #2 → Times.Never
- AckAsync → Times.Once
- SseStream.Publish(e2) → Times.Never
- "GenAI Result Listener stopped" Information log present (clean break, not throw)
- No Error-level logs (proves it was a break, not a generic-catch rethrow)
Remaining uncovered line: <ExecuteAsync>d__5 L34 (closing brace of
`catch (OperationInterruptedException) when (...no queue...)`). That brace is
the leave-instruction sequence point for a catch body whose last statement is
`await Task.Delay(Timeout.Infinite, stoppingToken)`. Task.Delay with Infinite
has no normal-return path — every reachable case throws OperationCanceledException
out of the catch — so the leave target at L34 is unreachable Roslyn-emitted
state-machine noise. Documented as phantom; not chased.
No production code touched.
ANcpLua
added a commit
that referenced
this pull request
May 16, 2026
…k (L20-22)
Drives PaperlessREST/Features/EventProcessing/Presentation/GenAiResultListener.cs
from 58/61 (95.1%) to 60/61 (98.4%) on the CI-aligned dotcov metric. The state
machine <ExecuteAsync>d__5 moves from line-rate 88% / branch-rate 50% to
line-rate 96% / branch-rate 100%.
Adds one fact + one helper iterator to ListenerLifecycleTests.cs:
GenAi_ExecuteAsync_TokenCancelledBetweenYields_BodyBreakCheckFires hits the
`if (stoppingToken.IsCancellationRequested) { break; }` block inside the
await-foreach. The pre-existing StoppingTokenCancelled test cannot reach it —
cancellation through the [EnumeratorCancellation] token short-circuits the
iterator before the loop body re-enters, so the body's IsCancellationRequested
check never fires. The new YieldAfterCancel<T> iterator deliberately ignores
[EnumeratorCancellation], yields the second event after the test cancels the
CTS, and forces the body-internal check to trip + break. Asserts:
- DocumentService.UpdateDocumentSummaryAsync for event #2 → Times.Never
- AckAsync → Times.Once
- SseStream.Publish(e2) → Times.Never
- "GenAI Result Listener stopped" Information log present (clean break, not throw)
- No Error-level logs (proves it was a break, not a generic-catch rethrow)
Remaining uncovered line: <ExecuteAsync>d__5 L34 (closing brace of
`catch (OperationInterruptedException) when (...no queue...)`). That brace is
the leave-instruction sequence point for a catch body whose last statement is
`await Task.Delay(Timeout.Infinite, stoppingToken)`. Task.Delay with Infinite
has no normal-return path — every reachable case throws OperationCanceledException
out of the catch — so the leave target at L34 is unreachable Roslyn-emitted
state-machine noise. Documented as phantom; not chased.
No production code touched.
ANcpLua
added a commit
that referenced
this pull request
May 16, 2026
…ener lifecycles; delete unused factories (#19) * chore(rest): delete unused RichProblemDetailsFactory/ErrorMetadataExtensions — superseded by DocumentErrors * refactor(rest): delete unused DocumentErrors/BatchErrors factories (no callers) Verified by grep across PaperlessREST + PaperlessServices that these factories have zero production references outside their own files: DocumentErrors (deleted): - StorageTimeout, StorageServerError, StorageConnectionFailed: only used in DocumentServiceErrorMappingTests via the inline Error.Unexpected(...) calls in TryMapStorageException, not via the factory methods. The string codes are duplicated between the factory and the inline calls; tests assert against the inline codes. - StorageUnavailable, SearchUnavailable, StorageFailed, DeleteFailed, InvalidStateTransition, MessageBrokerUnavailable (both overloads): only referenced in XML doc comments in DocumentEndpoints.cs, never invoked. BatchErrors (deleted entirely): - PathRequired, InvalidPath, PathsNotDistinct, InvalidTimeZone: zero references anywhere. BatchOptions validation is handled inline in ServiceCollectionExtensions via AddOptionsWithValidateOnStart. Per CLAUDE.md: 'delete a file outright when the codebase is healthier without it.' Brings DocumentErrors.cs to 100% (only Used factories remain) and removes BatchErrors as dead code. * test(rest): cover GenAi/Ocr listener ExecuteAsync lifecycle branches * test(services): cover SearchIndexService concurrency + ServiceCollectionExtensions InitializeAsync had two uncovered branches inside the lock that the existing SearchIndexIntegrationTests could not exercise because the shared fixture populates the static `s_initializedIndices` cache before the first test runs. Drives them via two new integration tests that mint a fresh `test-{Guid.NewGuid():N}` index per test and construct their own SearchIndexService (DI gives the fixture-bound singleton): - Branch (b): two concurrent IndexDocumentAsync calls on a unique index race the outer ContainsKey check, the semaphore serializes them, and the second caller hits the inner ContainsKey and short-circuits. Proved by asserting the "Created Elasticsearch index" log fires exactly once via FakeLogger. - Branch (c): pre-create the index out-of-band, then assert InitializeAsync finds Exists == true and emits no create-log. The document is still indexed successfully against the pre-existing index. A third test pins the "don't fake it" contract for null createdAt: the field is absent from the persisted _source rather than substituted with processedAt. PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs's no-arg AddOcrServices() and AddGenAiServices(IConfiguration) were unreachable from the integration fixture (which uses the IConfiguration overload). A new unit-test class covers them in isolation, plus the MinIO endpoint parsing branches (schemeless host:port vs. full URI) and UseSsl=true. * test(rest): cover Host.Extensions to 100% — ContractViolation, TypedErrorOr, OpenApi, ServiceCollection ContractViolationException + ContractViolationDiagnostics + ErrorDetail (was 0%): - Every factory: ForNotFoundOnly, ForValidationOnly, ForNotFoundOrConflict, ForCrudOperation, For (custom params). - BuildMessage single-error and three-error branches (verifies the literal "(+ 2 more error(s))" suffix). - GetDiagnostics round-trip including AllErrors ordering and metadata population (present + null). - Record equality holds when array-typed members reference the same instance, and fails when arrays differ (documents synthesized equality's reference semantics on T[]). - with-expressions for both records. - CallerMemberName default for ForNotFoundOnly. TypedErrorOrAsyncExtensions (was 34.6% line / 15.8% branch): - Sync ErrorOr<T>.ToOkOr404: success, NotFound, ContractViolation on non-NotFound. - Sync ErrorOr<Deleted>.ToNoContentOr404: success, NotFound, ContractViolation on non-NotFound. - Task<ErrorOr<T>>/Task<ErrorOr<Deleted>> overloads (delegating to the ValueTask state machines exercise both layers). - ToAcceptedAtRouteOrProblem: success path, Validation grouping by Code, Failure → 500 (kebab-case URN + camelCase metadata extensions), Unexpected → 503 with retryAfter default 30 / metadata override / RetryAfter-key exclusion, unhandled ErrorType → ContractViolation with [Validation, Failure, Unexpected]. - Null / empty / populated Metadata variants for both Failure and Unexpected. - ToKebabCase exercised via assertions on document.-storage-failed, plain.-failure, document.-storage-unavailable, i-pad-or-not, simple (covers leading-lowercase no-dash + internal-uppercase dash branches). OpenApiMetadataExtensions (was 75%): - ProducesNotFound, ProducesConflict, ProducesServiceUnavailable, ProducesGetByIdErrors, ProducesDeleteErrors(canConflict=false/true), ProducesWriteErrors, ProducesDocumentUploadErrors — assert via IEndpointRouteBuilder.DataSources materialization so the Finally callbacks actually run and metadata is observable. - ProducesDeleteErrors returns the same builder instance for chaining. ServiceCollectionExtensions EnsureStorageBucketAsync (was 42.86%): - Bucket exists → MakeBucket never called. - Bucket missing → MakeBucket invoked + LogInformation fires. - Race condition → ArgumentException "already owned" swallowed + LogDebug "already exists" fires. - ArgumentException without "already owned" rethrows. ServiceCollectionExtensions RegisterRecurringJobs: - AddOrUpdate called with JobId, cron, and timezone from BatchOptions. - LogInformation includes JobId/cron/tz. ServiceCollectionExtensions property accessors: - Minio, MinioOpts, BatchOpts, DbFactory return the registered instances. WebApplication.IsDev — Development=true / Production=false. Remaining: MapEndpoints' if(app.IsDev) branch (lines 34–36, 42–43) covers five lines that require a fully-wired WebApplicationFactory in Development environment (Hangfire dashboard, Scalar, OpenAPI). The integration tests spin Test environment by design. Leaving these uncovered for now; they are host-only wiring and exercised in dev runtime. * test(rest): cover DocumentService/ReportProcessor/UploadRequest gaps; delete unreachable XmlException branch DocumentService.cs gaps closed (100% line/branch): - UploadDocumentAsync: added test for unknown storage exception type that TryMapStorageException returns null for; asserts the original exception propagates uncaught (covers the `throw;` re-raise branch). - ProcessOcrResultAsync: added two tests for the transitionResult.IsError short-circuit (lines 148-151): one with an already-Completed document (Document.CannotComplete), one with an already-Failed document (Document.CannotFail). Asserts MockBehavior.Strict on the repository to prove UpdateAsync is NOT called when the state transition fails. ReportProcessor.cs gaps closed: - Added ProcessAsync_DateWithTimezone test: '2024-01-15+02:00' satisfies xs:date schema validation but fails DateOnly.TryParseExact('yyyy-MM-dd'), exercising the InvalidDate factory at lines 100-103. - DELETED the `catch (XmlException ex)` branch (lines 125-127) as unreachable: XmlSerializer.Deserialize wraps both XmlException and XmlSchemaException as InvalidOperationException before the catch chain sees them. Verified by writing a test against empty/malformed content and observing it always hits InvalidOperationException → InvalidSchema. - DELETED the corresponding ReportErrors.InvalidXml factory (sole caller was the deleted catch block). DTOs.cs (UploadDocumentRequest) gap closed: - New UploadDocumentRequestDtoTests.cs covers the synthesized record copy-constructor used by 'with' expressions (was the 50% uncovered half; the File property is exercised by every upload test). All 350 tests in PaperlessREST.Tests pass. * test(services): cover SearchIndexService catch block via ThrowExceptions(true) The existing SearchIndexServiceTests use ThrowExceptions(false), which routes transport failures through LogIndexResult's invalid-response warn path rather than the catch block at lines 57-61. Production wires the ElasticsearchClient with .ThrowExceptions() (true), so the catch IS exercised in production but was dead under the current test setup. Adds IndexDocumentAsync_WhenClientThrows_LogsWarningAndSwallowsException: constructs a fresh client with ThrowExceptions(true) against the unreachable host and asserts the catch-block log line ("Failed to index document...") fires with the underlying TransportException attached, while IndexDocumentAsync still completes without throwing. This is the path that protects OCR processing when Elasticsearch is genuinely unavailable in production. * test(rest): cover ServiceCollectionExtensions to 100% — IsDev, ProblemDetails, Hangfire, OpenApi, ApiExplorer lambdas Drives PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs from 41/46 (89.1%) to 46/46 (100% line + 100% branch) on the CI-aligned dotcov metric. Adds 9 facts to ServiceCollectionExtensionsTests.cs covering the previously- uncovered configuration lambda bodies that ASP.NET Core only invokes through its options pipeline: - MapEndpoints_WhenIsDev_RegistersDevelopmentOnlyRoutes — IsDev=true branch (MapOpenApi + Scalar + Hangfire dashboard) read off IEndpointRouteBuilder.DataSources - MapEndpoints_WhenNotDev_OmitsDevelopmentOnlyRoutes — IsDev=false branch - MapEndpoints_WhenIsDev_ScalarConfigureCallback_SetsTitleServersAndTheme — reflects into the Scalar request-delegate's captured configure Action since Scalar defers options to HTTP-request time - AddDependencies_ProblemDetailsCustomization_PopulatesTraceIdAndInstanceFromHttpContextWhenNoActivity — Activity.Current == null branch, asserts fallback to HttpContext.TraceIdentifier - AddDependencies_ProblemDetailsCustomization_UsesActivityIdWhenAvailable — Activity.Current != null branch, asserts Activity.Id wins over TraceIdentifier - AddDependencies_HangfireServerOptions_SetWorkerCountAndServerName — invokes only the BackgroundJobServerHostedService factory (to avoid resolving RabbitMQ listeners that would dial localhost), asserts WorkerCount == ProcessorCount and ServerName == "{MachineName}-{32hex GUID}" - AddDependencies_OpenApiCreateSchemaReferenceId_ReturnsNullForEnumAndDefaultForOther — enum → null, POCO → OpenApiOptions.CreateDefaultSchemaReferenceId default - AddDependencies_OpenApiDocumentTransformer_SetsTitleVersionAndDescription — extracts DelegateOpenApiDocumentTransformer._documentTransformer and invokes it on a fresh OpenApiDocument; asserts exact "Paperless OCR API" / "v1" / description - AddDependencies_ApiExplorerOptions_SetsGroupNameFormatAndSubstituteApiVersionInUrl — asserts exact "'v'VVV" + SubstituteApiVersionInUrl == true CreateWiredBuilder helper wires AddDependencies against in-memory IConfiguration and swaps PostgreSQL JobStorage for Hangfire MemoryStorage so IHostedService resolution does not require a running database. GetInlineProblemDetailsConfigure and GetOpenApiConfigure introspect the ServiceCollection for the production ConfigureNamedOptions<T>.Action so the actual production lambda is exercised rather than a copy. No production code touched. UnitTests: +9 facts, suite remains green. * test(rest): cover GenAiResultListener body-internal cancellation break (L20-22) Drives PaperlessREST/Features/EventProcessing/Presentation/GenAiResultListener.cs from 58/61 (95.1%) to 60/61 (98.4%) on the CI-aligned dotcov metric. The state machine <ExecuteAsync>d__5 moves from line-rate 88% / branch-rate 50% to line-rate 96% / branch-rate 100%. Adds one fact + one helper iterator to ListenerLifecycleTests.cs: GenAi_ExecuteAsync_TokenCancelledBetweenYields_BodyBreakCheckFires hits the `if (stoppingToken.IsCancellationRequested) { break; }` block inside the await-foreach. The pre-existing StoppingTokenCancelled test cannot reach it — cancellation through the [EnumeratorCancellation] token short-circuits the iterator before the loop body re-enters, so the body's IsCancellationRequested check never fires. The new YieldAfterCancel<T> iterator deliberately ignores [EnumeratorCancellation], yields the second event after the test cancels the CTS, and forces the body-internal check to trip + break. Asserts: - DocumentService.UpdateDocumentSummaryAsync for event #2 → Times.Never - AckAsync → Times.Once - SseStream.Publish(e2) → Times.Never - "GenAI Result Listener stopped" Information log present (clean break, not throw) - No Error-level logs (proves it was a break, not a generic-catch rethrow) Remaining uncovered line: <ExecuteAsync>d__5 L34 (closing brace of `catch (OperationInterruptedException) when (...no queue...)`). That brace is the leave-instruction sequence point for a catch body whose last statement is `await Task.Delay(Timeout.Infinite, stoppingToken)`. Task.Delay with Infinite has no normal-return path — every reachable case throws OperationCanceledException out of the catch — so the leave target at L34 is unreachable Roslyn-emitted state-machine noise. Documented as phantom; not chased. No production code touched. * test(rest): drop orphan tests left over from PR #16 rebase PR #16 added RichProblemDetailsFactoryTests.cs (exercises types deleted in efceded) and BatchAndReportErrorsTests.cs (exercises BatchErrors deleted in 4a1aa98 + ReportErrors.InvalidXml deleted in e97ff02). - Delete RichProblemDetailsFactoryTests.cs entirely — every symbol it references is gone (RichProblemDetailsFactory, ErrorMetadataExtensions). - Trim BatchAndReportErrorsTests.cs to the four surviving ReportErrors factories (FileNotFound, InvalidSchema, InvalidDate, InvalidGuid). The surface is the only test coverage of ReportErrors.* so it stays.
2 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
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
<PackageReference Version=…>out of the four production/test csproj files into rootVersion.propsvariables consumed byDirectory.Packages.props(ManagePackageVersionsCentrally=true). Mirrors the TourPlanner reference's config layout — single file owns the .NET 10 / EF Core 10 / Testcontainers / MTP version surface.Pipeline/Directory.Packages.propsso the NUKE bootstrap stays self-contained (matches the existingPipeline/Directory.Build.propsparent-isolation pattern).MartinCostello.Logging.XUnit.v3unified to0.7.1(patch bump; was split0.7.0vs0.7.1across the two test projects — CPM requires a single version).This is the first foundational PR in a series moving Paperless toward the TourPlanner C# pattern. Follow-ups will reshape the backend into the API / BL / Contracts / DAL split, rebuild
PaperlessUI.Angularfrom the TourPlanner-Angular frontend pattern, rebuildPaperlessUI.Reactfromqyl.dashboard, and replace test placeholders with OrbStack-backed real dependencies.Test plan
./build.sh Compile— Restore + Compile both succeed locallyBuild & Test (backend)green (Restore → Compile → UnitTests → IntegrationTests → Coverage → DotCov gate → Codecov)