feat(database): add roster and analytics schema - #27
MagicTheDev wants to merge 10 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ab2642014
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| uses_value := (entry->>'uses')::bigint; | ||
| triples_value := (entry->>'triples')::bigint; | ||
| IF triples_value > uses_value OR uses_value > attack_limit THEN RETURN false; END IF; |
There was a problem hiding this comment.
When an aggregate contains duplicate pet sets or several buckets whose combined uses exceed attack_count, this validator accepts it because each entry is checked independently. Since each attack contributes at most one whole pet set, the resulting daily analytics can report impossible totals; track the previous combo to reject duplicates/noncanonical ordering and ensure cumulative uses do not exceed attack_limit.
Useful? React with 👍 / 👎.
| ADD COLUMN siege_usage jsonb NOT NULL DEFAULT '[]'::jsonb | ||
| CHECK (jsonb_typeof(siege_usage) = 'array'); |
There was a problem hiding this comment.
When a writer supplies malformed or overcounted siege data, this check accepts any JSON array, including entries without numeric id/attacks fields, duplicate siege IDs, or totals above the row's attack_count. Because the column is documented as per-attack normalized siege counts, add a validator that enforces the entry shape, nonnegative counts, unique/canonical IDs, and a cumulative count no greater than attack_count.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 935d26ddd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ADD CONSTRAINT legend_daily_stats_siege_check | ||
| CHECK (public.item_usage_triples_within_attack_count(siege_stats,attack_count)), |
There was a problem hiding this comment.
When a cohort contains different selected siege machines, this reuses the generic item validator, which only checks each entry's uses independently. For example, two siege IDs can each report uses = attack_count, even though the canonical selected siege is mutually exclusive per attack, producing impossible daily totals; use a siege-specific validator that also caps cumulative uses at attack_count.
Useful? React with 👍 / 👎.
| IF first_value >= second_value OR triples_value > uses_value OR uses_value > attack_limit | ||
| OR hero_value < previous_hero | ||
| OR (hero_value = previous_hero AND first_value < previous_first) | ||
| OR (hero_value = previous_hero AND first_value = previous_first AND second_value <= previous_second) |
There was a problem hiding this comment.
When one hero appears with multiple equipment-pair buckets, each bucket may independently use the full attack_limit, so a five-attack aggregate can claim five uses for each of two mutually exclusive loadouts on the same hero. Track cumulative uses for the current heroId and reject a per-hero total above attack_limit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff23f25e74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ADD CONSTRAINT legend_daily_stats_pet_combos_check | ||
| CHECK (public.pet_combo_usage_triples_within_attack_count(pet_combo_stats,attack_count)); |
There was a problem hiding this comment.
Although migration 038 now caps cumulative uses, a row with three_star_count = 0 and pet_combo_stats = [{"petIds":[1],"uses":1,"triples":1}] still passes because the validator receives only attack_count and checks triples <= uses. Whole pet combos are mutually exclusive per attack, so their cumulative triples cannot exceed legend_daily_stats.three_star_count; otherwise the aggregate can report triple attacks while its star totals report none.
Useful? React with 👍 / 👎.
| if os.Getenv("CLASHKING_DISPOSABLE_TIMESCALE") != "1" || os.Getenv("TEST_DATABASE_URL") == "" { | ||
| t.Skip("requires disposable authoritative Goose schema") |
There was a problem hiding this comment.
In the checked workflows, schema-upgrade.yml invokes only the baseline-006, baseline-013, and baseline-018 profiles, while validate-repository.sh runs go test without a disposable database, so this new analytics test always skips in CI; TestPersonalArmyStorageMigration likewise always skips because it requires baseline-021. Add explicit retained-api and baseline-021 workflow invocations so the new constraint logic and populated 021-to-022 upgrade are actually exercised.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e4c66bad5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CREATE TABLE public.army_setup_daily_stats ( | ||
| day date NOT NULL, | ||
| league_tier_id integer NOT NULL CHECK (league_tier_id > 0), | ||
| rank_limit integer CHECK (rank_limit IN (200,1000)), |
There was a problem hiding this comment.
Include 100 in the allowed rank limits. Migration 030 adds the top_100 cohort to legend_daily_stats, and migration 034 attaches army-analysis readiness to those rows, but an analyzer cannot persist the corresponding setup observations because every row with rank_limit = 100 is rejected by this constraint. This can cause the top-100 analysis transaction to fail or leave that cohort permanently unclassified.
Useful? React with 👍 / 👎.
| ADD CONSTRAINT legend_daily_stats_siege_check | ||
| CHECK (public.legend_siege_usage_within_attack_count(siege_stats,attack_count)), | ||
| DROP CONSTRAINT legend_daily_stats_equipment_pairs_check, | ||
| ADD CONSTRAINT legend_daily_stats_equipment_pairs_check | ||
| CHECK (public.equipment_pair_usage_triples_within_attack_count(equipment_pair_stats,attack_count)); |
There was a problem hiding this comment.
Pass three_star_count into these final siege and equipment-pair validators and enforce it alongside attack_count. Currently a row with three_star_count = 0 accepts, for example, siege metadata containing {"uses":1,"triples":1}; equipment pairs have the same problem. Because siege choices are exclusive per attack and equipment-pair choices are exclusive per hero, their triple totals can otherwise contradict the row's authoritative star distribution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f57a895b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ADD CONSTRAINT legend_daily_stats_troops_check | ||
| CHECK (public.item_usage_triples_within_attack_count(troop_stats,attack_count)), | ||
| ADD CONSTRAINT legend_daily_stats_spells_check | ||
| CHECK (public.item_usage_triples_within_attack_count(spell_stats,attack_count)), |
There was a problem hiding this comment.
When a writer supplies troop or spell metadata for a row with fewer three-star attacks, these checks only bound uses by attack_count and triples by uses. For example, a row with three_star_count = 0 still accepts troop_stats = [{"id":1,"uses":1,"triples":1}], allowing analytics to contradict the authoritative star distribution. Each troop/spell entry's triples should also be capped by three_star_count without summing across items, since several items can occur in the same attack.
Useful? React with 👍 / 👎.
| -- Removing the save never removes the shared composition or retained history. | ||
| CREATE TABLE public.user_saved_armies ( | ||
| user_id text NOT NULL REFERENCES public.auth_users(user_id) ON DELETE CASCADE, | ||
| share_code text NOT NULL REFERENCES public.army_compositions(share_code) ON DELETE CASCADE, |
There was a problem hiding this comment.
When an army_compositions row is deleted, PostgreSQL must find every referencing saved-army row to enforce this cascade, but both available indexes lead with user_id; therefore each composition deletion scans the entire unlimited personal-army library. Add an index beginning with share_code so cleanup or batch deletion of canonical compositions does not become increasingly expensive or hold locks for a full-table scan.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a07172d655
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| group_count >= 0 AND clan_count >= 0 AND registered_player_count >= 0 | ||
| AND finalized_wars >= 0 AND archived_wars >= 0 AND archived_wars <= finalized_wars | ||
| ), | ||
| CONSTRAINT cwl_participation_townhall_check CHECK (jsonb_typeof(townhall_counts) = 'array'), |
There was a problem hiding this comment.
When a manual rebuild accidentally writes a value such as [null] or [{"level":"17","count":-1}], this constraint accepts it because it checks only the outer array type. The earlier CWL aggregate's cwl_town_halls_valid function validated entry keys, numeric types, level bounds, counts, and canonical ordering; without equivalent validation, malformed distributions can be retained and returned as analytics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 465ef44ad0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| entry jsonb; | ||
| total_uses bigint := 0; | ||
| BEGIN | ||
| IF attack_limit < 0 OR NOT public.item_usage_triples_valid(value) THEN RETURN false; END IF; |
There was a problem hiding this comment.
When a writer supplies siege_stats containing {"id":0,"uses":1,"triples":0}, item_usage_triples_valid accepts the entry and this validator only sums its usage, so the row passes despite representing no canonical selected siege. The equivalent normalized setup validator explicitly rejects siege_id = 0 in migration 038, and both columns describe the selected siege; apply the same positive-ID requirement here to prevent an invalid bucket from reaching Legend analytics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a671f095f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CONSTRAINT cwl_participation_hitrates_check CHECK ( | ||
| same_th_hitrates IS NULL OR jsonb_typeof(same_th_hitrates) = 'array' |
There was a problem hiding this comment.
When a rebuild writes malformed hit-rate data such as [null] or [{}], this constraint accepts it because it checks only the outer array type, and no later migration tightens this column. Such values can therefore be retained as analytics even though consumers cannot interpret them as hit-rate observations; validate each entry’s required fields, numeric types, bounds, and canonical ordering.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds the authoritative Goose migrations for personal armies, scoped roster signup and admission, Discord roster publication/webhooks, automation event offsets and capacity, legend daily metadata, CWL participation, and daily army setups. A forward migration removes the retired per-server link-token switch so new links require token proof. Migrations 037–041 add publication nonce reservations and tighten daily analytics JSON counts, including exclusive-use totals, three-star ceilings, and per-item troop/spell triple bounds. Migration 041 also indexes saved-army references by composition for FK cascades. Migration 042 restores structured validation of CWL town-hall distribution data; 043 rejects the zero sentinel as a stored Legend selected-siege ID; 044 validates CWL same-TH hit-rate entries. Previously applied migration files remain byte-for-byte unchanged in the final tree.
The branch also includes the local development workspace/CLI that had remained local, plus migration inventory, fixture-profile, schema tests, and documentation updates. The contributor-guide change already present on
mainwas skipped during rebase because its patch matched exactly.Daily setup
rank_limitintentionally remainsNULL(Overall), 1000, or 200. The experimentaltop_100cohort is Legend metadata only; both the active API setup endpoint and Tracking setup writer explicitly use the three supported setup cohorts.Validation
go test ./migrations ./schemafromdatabase/node --test scripts/with-test-timescale.test.mjsTestDailyAnalyticsUsageConstraintschecked valid, malformed, duplicate, unordered, zero-ID, and overcounted pet/siege/equipment data, exclusive three-star totals, and per-item troop/spell triples.TestCWLParticipationTownHallsandTestCWLParticipationHitRateschecked typed descending CWL entries and actual table constraints. The populated baseline-021 personal-army migration test also passed; the fixture-gated tests now run in CI.git diff --checkNo production migration has been run. Merge this schema PR before the dependent API and Tracking changes are deployed; API CI pins this PR's commit for isolated PostgreSQL tests.