Skip to content

Fix gamification tool payload duplication and dropped-data recovery (#1252) - #706

Merged
thomasluizon merged 4 commits into
redesign/mainfrom
fix/ticket-1252-streak-tool-payload
Oct 5, 2026
Merged

thomasluizon merged 4 commits into
redesign/mainfrom
fix/ticket-1252-streak-tool-payload

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Fixes thomasluizon/orbit-tickets#1252

The default gamification overview duplicated the achievement catalogue and could drop the data needed to answer a streak question. A Pro fixture with every active achievement earned and progress computed by the production calculator serialized to 19,026 characters against the 12,000-character cap before this fix.

GetGamificationOverviewTool.cs now keeps the catalogue in the achievements section when both sections are requested. Profile-only calls retain the catalogue, and the profile response is copied so the original query result is preserved. Each flag describes its data and default; the streak flag gives explicit arguments for a streak-only call. This keeps the existing payload types and query contracts while removing the duplicate catalogue.

ToolFailureSection.cs now tells the model to call again with narrower arguments after any dropped payload, including successful tool calls, without repeating the same arguments or exposing size, limits, or processing details. If narrowing still cannot read the data, it tells the person plainly what it could not read. The existing drop marker remains unchanged.

Regression coverage lives in ChecklistUserFactPlatformToolTests.cs and a separate new method beside the tool failure cases in PromptSectionTests.cs. The payload fixture derives its definitions and progress from AchievementDefinitions.Active and AchievementProgressCalculator.Compute, includes every earned record, and uses the tool serializer's camelCase and null-skipping options. It checks the default, all single-section calls, and all two-section combinations for the cap and exactly one complete catalogue while retaining profile and streak data.

Assumptions

  • When both profile and achievements are requested, keep the catalogue in the achievements section and an empty catalogue list in the copied profile; rejected changing HTTP DTOs or removing the catalogue from profile-only calls.
  • Retained profile data instead of constructing a card from streak fields alone: the shared schema and both client cards require level and XP.
  • Selected the latest complete payload instead of the latest successful payload, preventing an incomplete later read from hiding the card.

Test evidence

Before changing tests or implementation, on base 3f82b8d9 with the defect present:

  • dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~GetGamificationOverviewTool --nologo: exit 0, all 5 existing cases passed, including GetGamificationOverviewTool_SkipsQueriesWhenAllFlagsAreFalse and GetGamificationOverviewTool_ReturnsSuccessForAllSections.
  • dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~ToolFailureSection|FullyQualifiedName~ContinueWithToolResultsAsync_Drops' --nologo: exit 0, all 4 existing tool failure cases passed, including Build_DefinesTerminalFailureRules and Build_PreservesClarificationForAmbiguousRequests.
  • dotnet test tests/Orbit.Infrastructure.Tests --no-build --filter FullyQualifiedName~ContinueWithToolResultsAsync_OversizedPayload_SendsValidDropMarker --nologo: exit 0, the existing drop-marker case passed.
    After adding tests with the relevant implementation still unfixed:
  • dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~GetGamificationOverviewTool --nologo: exit 1, 5 existing cases passed and both new cases failed. GetGamificationOverviewTool_KeepsFullyEarnedProPayloadWithinLimit observed 19,026 > 12,000 with empty arguments. GetGamificationOverviewTool_DescribesFlagsAndStreakOnlyArguments failed because include_profile had no description.
  • dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~ToolFailureSection --nologo: exit 1, 4 existing cases passed and Build_RetriesDroppedPayloadWithoutExposingInternalLimits failed because the dropped-payload recovery and disclosure rules were absent.
    After the fixes:
  • dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~GetGamificationOverviewTool --nologo: exit 0, all 7 cases passed, including all flag combinations in the new payload regression.
  • dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~ToolFailureSection|FullyQualifiedName~ContinueWithToolResultsAsync_OversizedPayload_SendsValidDropMarker' --nologo: exit 0, all 6 cases passed, including the new recovery rule and the unchanged drop-marker regression.
    Broader verification used the committed changes. Each command below returned exit code 0:
    | Command | Exit code | Result |
    | --- | --- | --- |
    | env -u LANG dotnet build Orbit.slnx --nologo | 0 | 0 errors |
    | env -u LANG dotnet test tests/Orbit.Application.Tests --no-build --nologo | 0 | 5,444 passed, 0 failed, 0 skipped |
    | env -u LANG dotnet test tests/Orbit.Infrastructure.Tests --no-build --nologo | 0 | 3,280 passed, 0 failed, 0 skipped |
    | env -u LANG dotnet test --no-build --nologo | 0 | 9,417 passed across application, infrastructure, domain, and analyzer projects; 0 failed, 0 skipped |
    | env LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx --nologo | 0 | 0 errors |
    | env LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Application.Tests --no-build --nologo | 0 | 5,444 passed, 0 failed, 0 skipped |
    | env LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Infrastructure.Tests --no-build --nologo | 0 | 3,280 passed, 0 failed, 0 skipped |
    Unchanged tests passed with the defect present:
dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~StatusCardBuilderTests|FullyQualifiedName~ChecklistUserFactPlatformToolTests'
dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~PromptSectionTests|FullyQualifiedName~NotificationVoiceTests'

Results: 67 and 129 passed, respectively; both exit code 0.
Before the fix:

dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~Handle_RecommendedStreakRead_ReturnsValueInCard|FullyQualifiedName~GetGamificationOverviewTool_DescribesFlagsAndStreakOnlyArguments'
dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~Build_RetriesDroppedPayloadWithoutExposingInternalLimits'

Both exited 1. The three reply regression cases returned a null streak card; both strengthened guidance tests rejected include_profile=false.
After the fix, focused suites passed 78 application and 146 infrastructure tests. These include payload-cap and drop-marker coverage. Recovery retries were scripted at the AI port.
Full verification:

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

All four exited 0. Both builds had zero errors; each test run passed all 9,427 tests.

Manual steps

Deploy through GitHub Actions → orbit-api → Release API → Run workflow, dispatched from main:

  • Staging: environment=staging, branch=redesign/main after the fix is merged.
  • Production: environment=production, branch=main once the fix reaches main.

@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

The new streak-only guidance can leave card-capable web and mobile clients without the requested streak answer.

Reviewed changes: Reviewed the gamification payload deduplication, tool argument guidance, dropped-payload recovery prompt, and their regression coverage.

  • Catalogue deduplication: Combined requests retain one catalogue in the achievements section while preserving profile-only responses and the original query result.
  • Narrow-read guidance: Tool descriptions and the failure prompt recommend streak-only arguments and narrower recovery without exposing internal limits.
  • Regression coverage: All 13 focused tests passed; a separate production-tool/card reproduction passed with default arguments but failed with the newly recommended streak-only arguments.

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

Comment thread src/Orbit.Application/Chat/Tools/Implementations/GetGamificationOverviewTool.cs Outdated

@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 the changes since the prior Pullfrog review at b7424124, including the streak-card fix in d29c3832, and confirmed the resolved finding is addressed.

  • Preserved card data: Updated both the tool description and dropped-payload recovery example to retain profile data alongside streak information, satisfying the card's level and XP requirements.
  • Prevented partial-read shadowing: Selected the latest successful payload containing both profile and streak when building a streak card, without changing other payload consumers.
  • Verified regressions: Passed 17 focused application tests and 6 infrastructure tests, covering the recommended read, scripted recovery, a subsequent achievements-only read, payload limits, and the drop marker. The full suite was not rerun during this review.

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

@sonarqubecloud

sonarqubecloud Bot commented Oct 5, 2026

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit 86e7467 into redesign/main Oct 5, 2026
26 checks passed
@thomasluizon
thomasluizon deleted the fix/ticket-1252-streak-tool-payload branch October 5, 2026 08:37
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