Skip to content

Fix Astra habit resolution and approval previews - #796

Merged
thomasluizon merged 12 commits into
redesign/mainfrom
thomasluizon/ticket-1568-astra-resolution
Oct 20, 2026
Merged

thomasluizon merged 12 commits into
redesign/mainfrom
thomasluizon/ticket-1568-astra-resolution

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Fixes thomasluizon/orbit-tickets#1568

Astra resolves the account's existing habits before preparing writes, returns one approval for repeated log or tag calls, and asks about missing names without an empty preview. Reading goals link to the resolved habit, and breakdowns create children under the existing parent.

Approach

  • HabitNameMatcher, QueryHabitsTool, and BulkHabitSelection normalize case, accents, partial names, and the leitura/ler activity alias. Search runs after the repository's user-scoped SQL query. The previous string.Contains(..., StringComparison.OrdinalIgnoreCase) predicate throws on the relational provider; the unchanged repository-mock tests missed that path. Other chat tools using this comparison already filter materialized collections.
  • ProcessUserChatCommand.Resolution, the tool loop, and PendingAgentOperationStore resolve write targets, refuse existing or previously proposed creates, combine date-range logs and tag assignments, and carry missing names into one reply. Duplicate protection reads server-owned pending proposals, including their parent scope. Tag requests do not turn missing habits into new creates.
  • BulkCreateHabitsTool accepts an existing parent, BulkLogHabitsTool accepts all requested dates in one call, and AssignTagsTool accepts multiple existing targets. They reuse existing command validation and domain guards. Child creation and tag replacement retain transaction boundaries. The executor refuses empty selection previews and returns preview_unavailable when preview generation fails, and the response accumulator also excludes empty or repeated approvals.
  • AgentArgumentPreview, PendingOperationChangePreviewer, and PendingOperationRevisionService canonicalize property order and nested JSON values. Approval fingerprints bind the affected records and referenced habits or tags. Root creation has no dependency on unrelated habits. Selecting one date from a range preserves the offered item identity.
  • Pending tool results return an explicit awaiting-confirmation status, proposed fields, resolved names, and item/habit/date counts. TurnFactSet reads those facts, and the prompt tells the model to describe proposals using the returned facts. A preview of twelve records now grounds both the two-habit and six-date counts without changing the grounding feature flag.

The stored argument column is jsonb. PostgreSQL documents that this type does not preserve object key order: JSON types. Equivalent reordered arguments reproduced different preview fingerprints before the fix. This is the storage-level explanation inferred from the column mapping and regression; the tests do not connect to staging.

No REST response field was removed, renamed, or retyped. The tool arguments are additive. The client renderer and model configuration are outside this change. Related thomasluizon/orbit-tickets#19 and thomasluizon/orbit-tickets#246 remain separate.

Assumptions

  • Use the existing chat resolution and approval services rather than add a separate endpoint or change the client renderer.
  • Use persisted pending operations for cross-turn proposal checks rather than trust client-supplied assistant history as server state.
  • Represent each habit/date write as a selectable preview item rather than hide multiple dates inside a single date field.
  • Bind linked habits and existing parents as approval dependencies rather than bind the whole account's habit collection.

Used the existing HeldWriteTestContext for range regressions instead of adding another mocked execution harness.

Test evidence

The account-shaped fixtures use Beber água, Ler 10 minutos, and Academia, with no meditation habit. Fake model responses drive the application handler; relational query tests use the repository's SQLite compatibility factory. No live model, application server, browser, or production database was used.

Unchanged tests with the defects present:

  • dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~QueryHabitsToolTests --verbosity quiet: 41 passed.
  • dotnet test tests/Orbit.Application.Tests --no-build --filter 'FullyQualifiedName~PendingOperationChangePreviewerTests|FullyQualifiedName~AgentArgumentPreviewTests' --verbosity quiet: 45 passed.
  • dotnet test tests/Orbit.Application.Tests --no-build --filter FullyQualifiedName~ProcessUserChatCommandHandlerTests --verbosity quiet: 241 passed.

Strengthened regressions, observed before their fixes:

  • dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~AstraHabitQueryRegressionTests --verbosity quiet: all three account-shaped search cases failed with the relational translation exception. The same filter passed after the search fix.
  • dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~PreviewAsync_Create_JsonPropertyOrderDoesNotChangeApproval|FullyQualifiedName~PreviewAsync_NoMatches_DoesNotOfferEmptyPreview' --logger 'console;verbosity=normal': both failed, respectively on unequal fingerprints and an emitted empty preview. Both pass after the fix.
  • dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~Replay_ --logger 'console;verbosity=normal': the initial six new request replays failed on unresolved links, a recreated parent, duplicate/empty log previews, duplicate habit creation during tagging, and absent grounding facts. Two unrelated existing replay cases passed. The six new cases passed after the fix; coverage now includes thirteen application replay cases.
  • dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~Replay_PreviousCreateProposal --logger 'console;verbosity=normal': failed because a changed proposal for the same name reached approval. Passes with the persisted-proposal guard. A later strengthening of the same command also proved the shorter Yoga name failed against a pending Praticar yoga proposal before applying name matching to stored proposals; both cases now pass.
  • dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~Replay_TagMatchingHabits|FullyQualifiedName~Replay_LogPreview_Grounds' --logger 'console;verbosity=normal': both failed, on two tag approvals and ungrounded habit/date counts. Both pass after batching and returning those counts.
  • dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~PreviewAsync_GoalLink_OnlyLinkedRecordChangesInvalidateApproval --logger 'console;verbosity=normal': failed because changing the linked habit left the fingerprint unchanged. Passes after binding that dependency; an unrelated change preserves approval.
  • dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~ExecuteAsync_EmptySelection_NeverReturnsAnApproval|FullyQualifiedName~ExecuteAsync_CreateApproval_JsonStorageOrder' --logger 'console;verbosity=normal': the empty-preview boundary case failed and the create-approval case passed. Both now pass, including approval after an unrelated habit change.
  • dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~ReviseDateRange_KeepingOneDate --logger 'console;verbosity=normal': failed with stale_preview after selecting one offered date. Passed after preserving the offered identity.

The infrastructure suite initially had one failure: its old PreviewFailure_KeepsPendingConfirmation test expected approval after a preview exception. That fallback is now a retryable preview_unavailable failure with no pending approval. The updated AgentOperationExecutor_PreviewFailure_DoesNotOfferUnboundApproval test and the other 79 focused infrastructure cases pass.

The generic multi-round accumulation fixture now uses two distinct habit titles. Its former identical empty arguments represented the repeated operation that the server now refuses.

Final verification, all commands exited 0:

Command Result
dotnet build Orbit.slnx --verbosity quiet 0 errors; 19 warnings in existing package and API code
dotnet test tests/Orbit.Domain.Tests --no-build --verbosity quiet 759 passed
dotnet test tests/Orbit.Application.Tests --no-build --verbosity quiet 5,437 passed
dotnet test tests/Orbit.Infrastructure.Tests --no-build --verbosity quiet 3,902 passed
dotnet test tests/Orbit.Analyzers.Tests --no-build --verbosity quiet 32 passed

Total: 10,130 tests passed, none skipped. Build regenerated OpenAPI documents with no tracked diff. Pre-commit formatting, dash, timeless, root-path, and suppression checks passed. Added source has no bare narration comments.

Before the fix, the unchanged single-date test passed with this command, exit 0:

dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~ReviseDateRange|FullyQualifiedName~ReviseAsync_EditsOneLogDateWithoutChangingAnother'

The new ReviseDateRange_KeepingOneDate_PreservesTheOfferedItem cases each reproduced the stack overflow, exit 1:

dotnet test tests/Orbit.Application.Tests --no-build --filter 'FullyQualifiedName~DatedPendingOperationRevisionTests&DisplayName~False'
dotnet test tests/Orbit.Application.Tests --no-build --filter 'FullyQualifiedName~DatedPendingOperationRevisionTests&DisplayName~True'

After the fix and again after merging, this focused run passed all 202 tests, exit 0:

dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~DatedPendingOperationRevisionTests|FullyQualifiedName~PendingOperationChangePreviewerTests|FullyQualifiedName~PendingOperationRevisionServiceTests|FullyQualifiedName~HeldWriteRevisionTests'

Final verification:

  • dotnet build Orbit.slnx: exit 0, zero errors, before and after merging.
  • dotnet test tests/Orbit.Domain.Tests: exit 0, 797 passed.
  • dotnet test tests/Orbit.Application.Tests: exit 0, 5,464 passed.
  • dotnet test tests/Orbit.Infrastructure.Tests: exit 0, 3,949 passed.
  • Both regenerated OpenAPI documents match origin/redesign/main exactly.
  • Unchanged baseline: 71 Application tests and 65 Infrastructure tests passed despite the defects.
  • Strengthened regressions failed before fixes for parent visibility, duplicate questions, word boundaries, missing fingerprints, JSON ordering, batching, date limits, filtered metrics, replacement requests, and empty selections.
  • Red commands included:
    • dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~RoundThree_|FullyQualifiedName~Replay_HealthTag' --verbosity quiet
    • dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~RoundThree_ --verbosity quiet
    • Individual filters: RoundThree_FilteredMetrics, RoundThree_ExplicitCreate, RoundThree_PendingDuplicate, Replay_UnresolvedNames_NoEmptyPreviewAndOneQuestion, and Replay_LogSelectionRetainsCompletedHabitsFromTurnSnapshot.
  • The corrected ambiguity fixture passed afterward; a clean pre-fix run after correcting that fixture was not captured.
  • Final build: exit 0, zero errors. OpenAPI regeneration produced no changes.
  • Final focused tests: 304 Application and 167 Infrastructure passed.
  • Full projects, each run with dotnet test tests/Orbit.<project>.Tests --no-build --verbosity quiet, exited 0:
    • Domain: 797 passed.
    • Application: 5,507 passed.
    • Infrastructure: 3,961 passed.
  • All commit hooks passed.

dotnet build Orbit.slnx: exit 0, zero errors. OpenAPI regenerated without a content change. Commit hooks passed.

These focused commands ran before implementation changes and again with strengthened tests:

dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~ProcessUserChatCommandHandlerTests|FullyQualifiedName~PendingOperationChangePreviewerTests|FullyQualifiedName~PendingOperationRevisionServiceTests|FullyQualifiedName~AgentArgumentPreviewTests|FullyQualifiedName~QueryHabitsToolTests'

dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~PendingAgentOperationStoreTests|FullyQualifiedName~AgentChatWriteHoldTests|FullyQualifiedName~AiControllerTests'
Phase Application Infrastructure
Unchanged tests, defects present 391 passed, exit 0 139 passed, exit 0
Strengthened tests, defects present 19 failed, exit 1 5 failed, exit 1
Committed fixes, adding --no-build 407 passed, exit 0 144 passed, exit 0

Red-first observations, all passing after the fix:

Finding Test Observed failure before fix
1 RoundFour_SupersedeOneChild_PreservesTheMultiItemOperation Real store cancelled the entire batch.
2 ExecutePendingOperation_InitialPreview_BindsApprovalToTargetSet, MCP cases Held MCP approval failed the fingerprint check.
3 RoundThree_BulkDelete_UsesNormalizedTitleWordPrefixes Unaccented search failed to select água.
4 RoundFour_Search_MatchesNormalizedTitleWordPrefixes and matcher theory Partial title words failed to match.
5 RoundThree_LogCoalescing_UsesTurnSnapshotAndReturnsCompactFacts No calls reported merged_into_preview; five were expected.
6 RoundFour_MalformedCreateAfterDuplicate_DoesNotReachExecutor Malformed creation reached the executor.
7 RoundThree_CreateChildren_ShowsParentInEveryItemAndOnceInChanges Three duplicate parent summary rows appeared.
8 Both held-create field-order tests Title appeared last.
10 RoundFour_Replacement_UsesStoredProposalWithUnrelatedReplyCopy and copy theories Replacement depended on reply wording; requested copy and grammatical number failed.
11 RoundThree_UnavailablePreview_RefusesEveryWrite Returned preview_unavailable instead of stale_preview.

Finding 9 was an assertion correction: Replay_HealthTag_DoesNotRecreateExistingHabits now checks held operations and the suppression tool result. It passed before and after; no production failure is claimed for that assertion weakness.

The additional localized-error assertion failed before its fix using:

dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~RoundFour_MalformedCreateAfterDuplicate_DoesNotReachExecutor

Full suites on the committed code:

Command Passed Exit
dotnet test tests/Orbit.Domain.Tests 797 0
dotnet test tests/Orbit.Application.Tests 5,700 0
dotnet test tests/Orbit.Infrastructure.Tests 4,013 0

Manual steps

None.

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

Caution

Revising a date-range log approval can terminate the API process with a stack overflow. The inline finding includes a reproduced failure.

Reviewed changes across all 29 changed files, tracing habit resolution, approval construction and revision through execution.

  • Habit resolution: Normalized search and name matching, active-habit reuse, persisted proposal deduplication, and missing-name clarification.
  • Batched writes: Existing-parent child creation, multi-date logging, and transactional multi-habit tag assignment.
  • Approval integrity: Canonical fingerprints, dependency binding, selectable habit/date items, and rejection of empty or failed previews.
  • Proposal grounding: Awaiting-confirmation payloads, counts and proposed fields supplied to the model and grounding checks.
  • Verification: 49 focused application tests and 103 focused infrastructure tests passed. Changing the existing date-range revision test to use the new public dates argument shape crashed the test host; temporary changes were restored.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread src/Orbit.Application/Chat/PendingOperationChangePreviewer.cs Outdated
Preserve selected habit/date items and their offered identities across revision, refresh, and execution. Make expanded previews terminal and remove the original date range from revised approvals.

Refs thomasluizon/orbit-tickets#1568
Carry the habit schedule expansion and isolated agent audit writes into the Astra fix. Rebuild both OpenAPI documents and verify they match origin/redesign/main exactly.

Refs thomasluizon/orbit-tickets#1568

@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. The resolved stack-overflow finding is fixed, and no new High or Critical findings remain.

Reviewed changes since the prior Pullfrog review at 00d0fe30, with the current full diff read for context.

  • Removed recursive expansion: Expanded date ranges inline once and treated revised_items as terminal, rejecting malformed selections without reverting to the original range.
  • Canonicalized revisions: Removed the original dates array when saving selected habit/date items while preserving their offered identities.
  • Added regression coverage: Exercised the public date-range input through narrowing, date editing, repeated revision, refresh and execution of only the retained item.
  • Verified behavior: Passed 202 focused application tests and 109 focused infrastructure tests at the current head, with no failures or skipped tests. The full solution suite was not rerun during this review.

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

@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 High or Critical issues found.

Reviewed changes since the prior Pullfrog review at 1cddaa48, with all 36 files in the current full diff read for context.

  • Tightened habit resolution: Added whole-word matching, explicit ambiguity and duplicate questions, exact normalized create deduplication, and replacement of pending proposals.
  • Preserved creation intent: Distinguished new root routines from explicitly requested children and exposed the existing parent in each child preview.
  • Strengthened approval binding: Required nonempty fingerprint-bound previews for every held chat mutation and checked stored dependencies on direct confirmation-token execution.
  • Batched data access: Reused the complete turn snapshot for log selection, batched tag validation and execution, capped date arrays, and deferred history loading until query candidates match.
  • Compacted proposal facts: Returned proposed titles, values, counts and dates without sending full preview items back to the model.

Verification passed: 5,507 Application tests and 3,961 Infrastructure tests, with no failures or skipped tests. The full solution and Domain suite were not rerun during this review. The earlier stack-overflow thread remains resolved.

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

@thomasluizon

Copy link
Copy Markdown
Owner Author

Copy approval for round 4 (42c9b4fb), run through /second-opinion framed as a claimed copy defect, so DISAGREE means the copy ships.

Strings reviewed (en / pt-BR): the pending creation question ("You already have a preview waiting to create {name}. Approve it, or ask me to prepare a new one." / "Você já tem uma prévia aguardando para criar {name}. Aprove a prévia ou me peça para preparar uma nova."), the multi-item replacement question, and the singular and plural not-found questions.

Verdict: DISAGREE, medium confidence. The pt-BR strings use the app's own terms ("prévia", "hábito"), singular and plural agree ("este hábito / Qual" and "estes hábitos / Quais"), and no adjective refers to Astra. One optional polish noted ("aguardando aprovação"), not a defect.

@sonarqubecloud

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 High or Critical issues found.

Reviewed changes since the prior Pullfrog review at d529eb17, with all 39 files in the current full diff read for context.

  • Preserved batch proposals: Refused single-item replacement of multi-item create previews and based replacement decisions on persisted proposals rather than assistant reply wording.
  • Unified title search: Applied normalized, order-independent word-prefix matching to habit queries and bulk selections, with matching schemas and prompt guidance.
  • Clarified merged calls: Returned merged_into_preview for coalesced log/tag calls and repeated calls, with explicit instructions not to prepare another preview.
  • Rejected malformed creates: Checked all titles before duplicate bookkeeping and returned localized failed actions for invalid calls.
  • Refined preview presentation: Put titles first, showed the parent once in summary changes, and corrected singular/plural clarification messages.
  • Preserved confirmation behavior: Allowed fingerprint-free non-Chat confirmations while retaining stored fingerprint checks for Chat mutations, and distinguished stale previews from preview-generation failures.

Verification passed: 5,700 Application tests and 4,013 Infrastructure tests, with no failures or skipped tests. Focused runs also passed 417 Application and 194 Infrastructure tests. The full solution build, Domain suite, and live model flows were not rerun during this review.

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

@thomasluizon
thomasluizon merged commit 81d8401 into redesign/main Oct 20, 2026
26 checks passed
@thomasluizon
thomasluizon deleted the thomasluizon/ticket-1568-astra-resolution branch October 20, 2026 00:42
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