Repository navigation
Fix gamification tool payload duplication and dropped-data recovery (#1252) - #706
Conversation
There was a problem hiding this comment.
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.
gpt-6.1-sol | 𝕏
There was a problem hiding this comment.
✅ 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.
gpt-6.1-sol | 𝕏
|




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.csnow 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.csnow 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.csand a separate new method beside the tool failure cases inPromptSectionTests.cs. The payload fixture derives its definitions and progress fromAchievementDefinitions.ActiveandAchievementProgressCalculator.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
Test evidence
Before changing tests or implementation, on base
3f82b8d9with the defect present:dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~GetGamificationOverviewTool --nologo: exit 0, all 5 existing cases passed, includingGetGamificationOverviewTool_SkipsQueriesWhenAllFlagsAreFalseandGetGamificationOverviewTool_ReturnsSuccessForAllSections.dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~ToolFailureSection|FullyQualifiedName~ContinueWithToolResultsAsync_Drops' --nologo: exit 0, all 4 existing tool failure cases passed, includingBuild_DefinesTerminalFailureRulesandBuild_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_KeepsFullyEarnedProPayloadWithinLimitobserved 19,026 > 12,000 with empty arguments.GetGamificationOverviewTool_DescribesFlagsAndStreakOnlyArgumentsfailed becauseinclude_profilehad no description.dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~ToolFailureSection --nologo: exit 1, 4 existing cases passed andBuild_RetriesDroppedPayloadWithoutExposingInternalLimitsfailed 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:
Results: 67 and 129 passed, respectively; both exit code 0.
Before the fix:
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:
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:environment=staging,branch=redesign/mainafter the fix is merged.environment=production,branch=mainonce the fix reachesmain.