fix(s3aar): correct enrich lookup inputs + close empty-payload gap (#151 #152 #153 #154)#156
Merged
Merged
Conversation
#152 #153 #154) Four #150-review follow-ups, all on commands/s3aar.go / s3aar_test.go: - #151: pass the raw BattleMetrics name to enrichPlayer so the gamertag fallback queries the uncleaned in-game tag; the username branch still cleans. Stops inflated "verify manually" lines. - #152: validateProfile rejects a milpacs 2xx with empty rank/username or empty roster, routing such players to the⚠️ warning field instead of emitting a blank [URL='...'] [/URL] ghost forum line. EnrichFailed now means "usable enrichment", not merely "HTTP succeeded". - #153: assert EnrichFailed round-trips (value + json:"enrich_failed" tag) in the /s3aar debug JSON. - #154: cover the enrichPlayer UniformUrl-regex-failure branch (2xx with malformed UniformUrl) as a surfaced enrichment failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Fix inverted invariant + present-tense rationale in validateProfile doc comment (a non-failed player means "usable enrichment", and the ghost-line hazard it guards is the pre-fix motivation, not current behavior). - Fold the player name into validateProfile error strings so an enrichment failure stays attributable even if a future caller logs only the error. - Add TestRunS3aar_EmptyRosterOnlyTreatedAsFailure covering validateProfile's empty-Roster-only clause (the existing empty-fields test only trips the rank/username clause). - Tighten the #154 regex-branch test to assert the WARN log carries the UniformUrl error text, pinning that branch vs an identical-looking 404. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- gofmt: align trailing comments in s3aar_test.go (golangci-lint defaults
don't run gofmt, so this wasn't caught by the CI gate).
- Distinguish the two UniformUrl regex-miss errors by lookup origin
(username-lookup vs gamertag-lookup) so an operator can tell which profile
carried the malformed URL from the WARN log alone.
- Add TestRunS3aar_GamertagFallbackEmptyFieldsTreatedAsFailure, pinning the
validateProfile guard on the gamertag fallback branch (previously a copy
uncovered by any test — mutation-confirmed it fails when the guard is
dropped).
- Tighten the empty-fields / empty-roster WARN assertions to pin the specific
validateProfile clause ("unusable enrichment" / "empty roster") and the
attributed player name, matching the #154 test's precision.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bundles the four
ready-for-agentfollow-ups from the #150 review. All four converge oncommands/s3aar.go/commands/s3aar_test.go, so they were implemented as one sequential TDD pass to avoid self-collision.Issues
nametoenrichPlayer; the username branch still cleans, the gamertag fallback gets the uncleaned in-game tag. Stops inflated "verify manually" lines.RankFull/Username/Rosterwas treated as success → blank[URL='...'] [/URL]ghost forum line. NewvalidateProfileguard fails such payloads so the player joins theEnrichFailednow means "usable enrichment", not merely "HTTP succeeded".TestRunS3aar_DebugJSONOutputnow assertsEnrichFailedround-trips (value +json:\"enrich_failed\"tag) for one enriched and one 404 player.TestRunS3aar_UniformURLRegexFailurecovers the 2xx-with-malformed-UniformUrlbranch as a surfaced enrichment failure.Verification
Full CI gate re-run independently in the worktree:
golangci-lint0 issues,go mod tidyclean,go test ./... -race -coverpass, coverage floors met (commands 82.6% ≥ 81%, utils 88.1% ≥ 87%),go buildOK.Manual review gate
/s3aaris a user-facing command — per CLAUDE.md this needs a smoke test on the test guild before merge (CI can't exercise the live Discord gateway). The empty-payload and raw-tag paths are the behaviors to eyeball.🤖 Generated with Claude Code