Skip to content

Validate bulk create previews before approval - #703

Merged
thomasluizon merged 2 commits into
redesign/mainfrom
fix/ticket-1220-bulk-preview-validation
Oct 5, 2026
Merged

thomasluizon merged 2 commits into
redesign/mainfrom
fix/ticket-1220-bulk-preview-validation

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

An invalid bulk create batch previously showed a clean pending preview and failed after approval. The preview now carries each item's validator errors, and confirmation and execution reject the batch until its invalid items are corrected or removed.

Closes thomasluizon/orbit-tickets#1220

Changes

  • PendingOperationChangePreviewer.cs builds the command with the existing bulk tool parser and runs BulkCreateHabitsCommandValidator. It maps failures to the matching item and offers missing invalid fields for editing. Batch-level failures apply to every item.
  • BulkCreateHabitsTool.cs permits the preview parser to retain an empty title so the existing validator can report it. Execution parsing keeps its existing behavior. The quantity default from #1215 is not changed here.
  • AgentContracts.cs appends optional validationErrors entries containing field, errorCode, and error. The field is omitted for valid items, preserving their serialized shape and preview fingerprint. src/Orbit.Api/openapi.json contains the generated optional contract.
  • PendingOperationRevisionService.cs recomputes approval eligibility. AiController.cs checks it before issuing a confirmation token and before dispatching execution, including requests without a stored preview fingerprint.
  • Preview, revision, controller, and contract tests cover invalid quantities, missing titles, valid batches, legacy payloads, and approval recovery through editing or removing an item. The recovery tests use the real tool, command validator, and domain argument check.

This keeps validation rules in the existing validator and preserves the append-only response contract.

Assumptions

  • Use an optional per-item validationErrors list rather than a single error, preserving every failure returned by the validator.
  • Use the preview's snake_case field names rather than CLR paths such as Habits[1].FrequencyQuantity, so errors identify the editable preview field.
  • Reject invalid approval with the existing validation error response and HTTP 400 rather than adding a new error code or treating validation as a stale preview.

Test evidence

Unchanged test with the preview defect present:

env -u LANG LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~PendingOperationChangePreviewerTests.PreviewAsync_Create_UsesStableInputIndexes --verbosity minimal

Passed: 1 test. It did not exercise an invalid item.

New regression with the defect still present:

env -u LANG LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~PendingOperationChangePreviewerTests.PreviewAsync_Create_InvalidItemCarriesValidatorFieldError --verbosity minimal

Failed: 1 test, because the second item's serialized preview lacked validationErrors. The test uses the real parser and validator, not a mocked validation response. FluentValidation 12.1.1 returned PropertyName = Habits[1].FrequencyQuantity and ErrorCode = GreaterThanValidator; the passing test also verifies that the preview's error equals the real failure's ErrorMessage. These are the external fields the implementation reads, and this command re-derives their values.

The unchanged AiControllerTests.ConfirmPendingOperation_ReturnsConfirmationToken also passed before the approval fix as part of the first focused run. The stronger approval regression ran before that fix:

env -u LANG LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~AiControllerTests.PendingOperation_InvalidBulkCreateCannotConfirmOrExecute --verbosity minimal

Failed: all 4 cases returned OkObjectResult instead of rejecting the invalid batch. Cases cover confirmation and execution, with and without a stored fingerprint.

After both fixes:

env -u LANG LC_ALL=en_US.UTF-8 dotnet test --no-build --filter 'FullyQualifiedName~PendingOperationChangePreviewerTests|FullyQualifiedName~PendingOperationRevisionServiceTests|FullyQualifiedName~PendingOperationItemContractTests|FullyQualifiedName~AiControllerTests' --verbosity minimal

Passed: all 90 focused tests, including both regressions and valid approval paths.

Final verification:

env -u LANG LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx
env -u LANG LC_ALL=en_US.UTF-8 dotnet test

Build: 0 errors. Tests: 9,395 passed, 0 failed, 0 skipped across all four projects. Existing build warnings remain. Git hooks passed, and changed C# files contain no bare narration comments.

@sonarqubecloud

sonarqubecloud Bot commented Oct 5, 2026

Copy link
Copy Markdown

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

✅ No new issues found.

Reviewed changes Reviewed bulk-create preview validation, approval enforcement, revision recovery, and append-only contract compatibility across all changed files.

  • Preview validation: Uses the existing bulk parser and command validator to attach item-specific errors and expose missing invalid fields for editing.
  • Approval enforcement: Rechecks validator eligibility before confirmation and execution, including pending operations without stored preview fingerprints.
  • Contract compatibility: Adds optional validationErrors while preserving valid-item serialization and fingerprints; checked the sibling consumer's existing pending-item schema.
  • Regression coverage: All 90 focused preview, revision, controller, and contract tests passed locally. The full test suite was not rerun during this review.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

@thomasluizon
thomasluizon merged commit 7ba05b9 into redesign/main Oct 5, 2026
26 checks passed
@thomasluizon
thomasluizon deleted the fix/ticket-1220-bulk-preview-validation branch October 5, 2026 03:30
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