Skip to content

fix: recover Astra turns after invalid ask calls - #1002

Open
thomasluizon wants to merge 3 commits into
mainfrom
fix/ticket-1716-g9b-ask-recovery
Open

thomasluizon wants to merge 3 commits into
mainfrom
fix/ticket-1716-g9b-ask-recovery

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner

Invalid ask calls discarded subsequent model tool plans, producing chat.answer.empty even when the continuation requested a batch preview. Failed questions now return validation feedback and preserve the model's continuation within the existing round and token budgets. A valid, accepted question still ends the reply.

Tracks thomasluizon/orbit-tickets#1716.

Implementation

  • ProcessUserChatCommand.Ask.cs and .Tools.cs remove the continuation filter that discarded recovery reads and writes. The two-attempt limit applies at ask execution, so exhausting question retries does not discard other tool calls. Withheld-call feedback also permits recovery.
  • AskTool.cs explains when clear requests need no question and describes absent nullable fields, including the required questions array for a question form. Existing validators, domain guards, selection protections and write approvals remain authoritative. No message patterns select or rewrite tools.
  • FailedAskRecoveryReplayTests.cs replays captured null questions at the model-service port, then exercises the real read and preview producers. It covers a batch preview with composition on and off, supported and missing read block kinds, English and Portuguese text continuations, and the question retry limit. The obsolete assertion that every failed ask must suppress later writes is replaced by these recovery cases; valid-question and unresolved-selection tests remain in the suite.
  • StrictChatToolSchemaTests.cs verifies the actual installed SDK serialization. AnswerComposition.md records all added or changed model-facing copy in English and Brazilian Portuguese; AskValidation.md links the recovery contract. No person-visible fallback string or public DTO changes.

Test evidence

Before changing implementation or existing tests, on 3277ebfe:

dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~AskCorrectionReplay|FullyQualifiedName~GateEightAsk|FullyQualifiedName~Ask_EmitsNegotiatedPartAndIgnoresRepeatedAsk' --verbosity quiet

All 40 unchanged tests passed with the defect present, including AskCorrectionReplay_ClarificationRetryCannotDispatchAWrite, whose blanket suppression assertion encoded the defect.

After adding the recovery cases while implementation was still unchanged:

dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~FailedAskRecovery' --verbosity quiet
dotnet test tests/Orbit.Application.Tests --no-build --filter 'FullyQualifiedName~FailedAskRecovery_CapturedNullQuestionPreservesBatchPreview' --logger 'console;verbosity=normal'

The first command exited 1: four recovery cases failed and the two text cases passed. The second exited 1 for both composition modes, with the observed failure: expected an empty narrative beside the preview, but received Não consegui preparar uma resposta. Tente novamente. This proves the medical regression fails on the base revision for the reported reason.

After the fix:

dotnet test tests/Orbit.Application.Tests -c Release --no-build --filter 'FullyQualifiedName~FailedAskRecovery|FullyQualifiedName~AskCorrectionReplay|FullyQualifiedName~GateEightAsk|FullyQualifiedName~Ask_EmitsNegotiatedPartAndIgnoresRepeatedAsk' --verbosity quiet
dotnet test tests/Orbit.Infrastructure.Tests -c Release --filter 'FullyQualifiedName~StrictChatToolSchemaTests' --logger 'console;verbosity=normal'

Both exited 0: 46 handler cases and 27 schema cases passed. Valid accepted questions still issue one block and make no continuation call. The schema cases serialize the real ResponseTool through the installed SDK, confirm strict required properties and nullable types, and check that nullable ask fields explain null. Ask_WireSchemaExplainsNullableFields also failed before the description changes. The fixtures contain observed executed arguments, not guessed provider response fields; production adds no external response field reads.

Verification

  • dotnet build Orbit.slnx -c Release --verbosity quiet: exit 0, zero errors.
  • dotnet test tests/Orbit.Domain.Tests -c Release --no-build --verbosity quiet: exit 0, 1,215 passed.
  • dotnet test tests/Orbit.Application.Tests -c Release --no-build --verbosity quiet: exit 0, 9,196 passed.
  • dotnet test tests/Orbit.Infrastructure.Tests -c Release --no-build --verbosity quiet: exit 0, 6,601 passed and 14 skipped.
  • dotnet test tests/Orbit.Analyzers.Tests -c Release --no-build --verbosity quiet: exit 0, 40 passed.
  • python3 .github/scripts/openapi_artifacts.py freshness: exit 0. All four generated publication artifacts match committed generation, so there is no artifact diff to commit.
  • node tools/capability-inventory.mjs --check: exit 0.
  • Commit hooks and changed-file comment checks passed. The coordinator owns deployment, the live feat: add calendar dismiss endpoint #26 rerun and CI waiting.

Assumptions

  • The later gate 9b note defines this round's scope; rejected reopening historical gate classes already handled on the base branch.
  • Captured executed calls are replayed as internal AiResponse plans, consistent with existing fixtures; rejected inventing raw provider envelopes or claiming captured call-round boundaries.
  • The medical gate 9b report retains only the failed ask, so its recovery uses the unchanged batch from Fixtures/GateEight/composition-medical-batch-pt-BR-r3.json; rejected presenting that batch as the unobserved gate 9b continuation. The compound replay uses the actual captured query_habits and query_goals recovery arguments.

Review round 1

  • Pullfrog P1 (a failed selection was forgotten before write recovery): every ask now keeps the option ids that are owned records in the account snapshot as unresolved selection state (PreserveAskRecordSelection in ProcessUserChatCommand.Ask.cs), and the write guard reads one SelectionCandidates list that both account reads and asks fill (ProcessUserChatCommand.Selection.cs). A write whose target is one of those candidates is withheld until the person answers; writes to other records still become previews, and the batch preview and compound read replays stay green. Typed tool arguments and the account snapshot only; nothing reads the person's words.
  • Regression FailedAskRecovery_RecordChoiceWithholdsOnlyCandidateWrites: two owned habits with the same title, a malformed choice with both real ids, then log_habit for one candidate with no query_habits. It failed on 1fa89d4f (an unchosen log_habit preview was held) and passes now; the unrelated-target case stays a held preview.
  • Base merge of API main 1535efea (orbit-api#1000 and Propose Astra restart plans without a selection question #1001) in the same commit.
  • Verification on 875c9bc4 by exit code: Release build 0, Domain 1,215, Application 9,218, Infrastructure 6,623 (14 skipped), analyzers 40, OpenAPI freshness 0, capability inventory 0. No new person-visible or model-facing string.

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

Important

Failed-ask recovery can bypass an unanswered target selection and prepare a preview for a model-guessed record.

Reviewed changes across both commits, including the continuation loop, question-attempt limit, tool descriptions, captured replays, and strict SDK schema tests.

  • Recovery flow: Rejected asks now preserve normal tool continuations, while accepted questions still end the reply and question attempts remain capped.
  • Model guidance: Ask descriptions clarify nullable fields, required question inputs, and when clear requests need no clarification.
  • Regression coverage: Replays cover batch previews, compound reads, text answers, and exhausted retries; bilingual documentation records the changed guidance.
  • Verification: All 248 ask/selection tests and 27 strict-schema tests passed, but a temporary targeted replay reproduced the unresolved-selection regression described inline.

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

Comment thread src/Orbit.Application/Chat/Commands/ProcessUserChatCommand.Tools.cs

@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 since the prior Pullfrog review at 1fa89d4f, focusing on the selection-state fix in 875c9bc4 and its interaction with failed-ask recovery.

  • Preserved unresolved selections: Retained owned habit option IDs from rejected asks in the shared selection state, withholding candidate-dependent writes before preview preparation while allowing unrelated previews.
  • Added regression coverage: Exercised direct malformed choices without a preceding habit query, covering composition enabled and disabled, English and Portuguese, and unrelated-target recovery.
  • Updated guidance: Documented the selection boundary and checked the merged-main ask instructions alongside the recovery flow.
  • Verified behavior: Passed all 254 ask/selection tests and all 27 strict-schema tests. Confirmed the previously resolved finding is substantively fixed; the full solution suite was not rerun in this review.

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

@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
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