Skip to content

docs(adr): configurable chunked $batch design (ADR-0004)#146

Merged
Mikeoso merged 1 commit into
mainfrom
docs/adr/batch-design
Jul 2, 2026
Merged

docs(adr): configurable chunked $batch design (ADR-0004)#146
Mikeoso merged 1 commit into
mainfrom
docs/adr/batch-design

Conversation

@Mikeoso

@Mikeoso Mikeoso commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

Records the v3.0.0 batch design (shipped in #145) as ADR-0004 and adds it to the docs/adr index.

Captures the non-obvious rationale so it isn't re-litigated:

  • Why hand-rolled multipart $batch — PanoramicData's changeset has no composite/anonymous-key support (re-verified on 10.0.84); uniform path covers every key shape incl. cross-company / no-DataAreaId.
  • Why Atomic/ContinueOnError + auto-chunking — D365 F&O caps a $batch at 200 operations; bulk imports must split, so atomicity is per-chunk, not per-dataset.
  • Why breaking Result<BatchOutcome> — per-item import error analysis (Index/ChunkIndex/Key/status/D365 error).
  • Why ContinueOnError is per-item today — the single-$batch Prefer: odata.continue-on-error transport is undocumented for F&O; deferred behind a live-JFI smoke run.

Risks / rollback

Docs-only (one new ADR file + index row). No code, no API surface change. Rollback = revert the commit.

Reviewer focus

Accuracy of the recorded decision against the merged #145 implementation.

🤖 Generated with Claude Code

Capture the v3.0.0 batch decision: uniform hand-rolled multipart $batch,
Atomic/ContinueOnError modes, auto-chunking under D365's 200-op cap, and the
breaking Result<BatchOutcome> shape — so the per-chunk atomicity and deferred
continue-on-error transport are not re-litigated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@claude claude 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.

Overview

Docs-only PR: adds ADR-0004 recording the v3.0.0 configurable chunked $batch design (shipped in #145) and indexes it in docs/adr/README.md. No source, test, or config files touched — risk is effectively zero (worst case is a stale/inaccurate historical record, not a runtime defect).

I cross-checked every concrete claim in the ADR against the actual merged implementation rather than taking it on faith:

  • BatchFailureMode (Atomic/ContinueOnError), BatchOptions, ODataSettings.Batch.{FailureMode,MaxOperationsPerChunk,StopOnFirstFailedChunk} — confirmed in IntegratoR.OData/Domain/Settings/ODataBatchSettings.cs and IntegratoR.Abstractions/Common/Batch/.
  • MaxOperationsPerChunk default 150, validator range 1..200 — confirmed in ODataSettingsValidator.cs:91-95 and ODataBatchSettings.cs:27.
  • StopOnFirstFailedChunk default true, honoured in Atomic mode — confirmed in ODataService.cs:643.
  • ODataService<T> ctor gaining an optional trailing IOptions<ODataSettings>? settings = null — confirmed at ODataService.cs:46-50; correctly characterised as a binary (not source) break.
  • The $batch POST riding the named "ODataClient" HttpClient's Polly policy but deliberately not wrapped in the exception handler's operation-retry — confirmed verbatim, the code even carries the same rationale as an inline comment (ODataService.cs:595-596).
  • GitVersion.yml next-version: 3.0.0 — present with a comment explaining the MAJOR bump.
  • Referenced test files (ODataBatchWireTests, ODataClientAdapterAtomicBatchTests, ODataServiceBatchChunkingTests) and wiki/Known-Limitations.md — all exist and are consistent with the ADR's description (the wiki page states the same 150/200/StopOnFirstFailedChunk facts).
  • ADR formatting (Status/Date header, section structure) matches ADR-0001/0002/0003, and the README table row is appended correctly.

No discrepancies found between the ADR's narrative and the shipped code.

Findings

None. No Blocker, Major, Minor, or Nit findings — this is an accurate, well-scoped documentation-only change with no code, no hard-rule surface, and no test-adequacy question (nothing behaviour-changing was added).

Stages run

  • Stage 1 (correctness/hard-rules/architecture/tests): ran — verified ADR accuracy against source, see above. Not applicable: no Result<T>/ConfigureAwait/CancellationToken/British-spelling/CPM/var/serialiser-lockstep surface exists in a markdown-only diff.
  • Stage 2 (security): skipped — flag was false, and there is no auth/secret/header/logging surface in this diff (pure prose, no code).
  • Stage 3 (API compatibility): skipped — flag was false; this PR touches no published IntegratoR.* API surface (the ADR documents an already-shipped, already-versioned breaking change from #145/6e779b3, it doesn't introduce one).
  • Mechanical checks (dotnet build, dotnet test, dotnet format --verify-no-changes, vulnerable-package scan) live in build.yml and were not recomputed here; the build check reports SUCCESS on this PR.

Verdict

approve

@Mikeoso
Mikeoso merged commit fef0e52 into main Jul 2, 2026
3 checks passed
@Mikeoso
Mikeoso deleted the docs/adr/batch-design branch July 2, 2026 12:32
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.

1 participant