classification #7 - add unit tests for multi-label support#933
Open
zale144 wants to merge 12 commits into
Open
classification #7 - add unit tests for multi-label support#933zale144 wants to merge 12 commits into
zale144 wants to merge 12 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 3 files
Architecture diagram
sequenceDiagram
participant W as Worker / Forecaster
participant M as Emissions Module
participant LR as Epoch Label Registry
participant IS as Inference Synthesis
participant K as Emissions Keeper (DB)
participant R as Rewards Module
Note over W,R: Multi-Label Inference & Rewards Flow
W->>M: Submit Inference / Forecast
M->>LR: NEW: Register/Validate Labeled Values
LR-->>M: Verified Label Space (ID mapping)
M->>K: Store Multi-Label Inferences
Note over M,R: End of Epoch / Inference Synthesis Trigger
M->>K: Fetch Inferences & Regrets
K-->>M: Labeled Data Bundles
M->>IS: CalcWeightsGivenWorkers(Regrets)
IS->>IS: Apply P-Norm/C-Norm scaling
IS-->>M: Regret-Informed Weights
M->>IS: GetCombinedInference(Weights, LabeledValues)
alt NEW: Multi-Label Aggregation
IS->>IS: Align values to Label Registry
IS->>IS: Weighted Average per Label
else Single-Label (Legacy)
IS->>IS: Default to single unlabeled entry
end
IS-->>M: Network Inferences (Combined & Naive)
M->>K: NEW: Store Labeled Network Inference Bundle
Note over M,R: Reward Distribution
M->>R: CalculateRewards(Network Inferences, Stake)
R->>K: Update Reputer Stake (Authority)
R->>K: CHANGED: Distribute Worker Rewards (Multi-label performance)
K-->>M: Success / Updated Balances
There was a problem hiding this comment.
4 issues found across 16 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="x/emissions/keeper/inference_synthesis/weights_test.go">
<violation number="1" location="x/emissions/keeper/inference_synthesis/weights_test.go:1">
P2: This file-level `nolint:exhaustruct` suppresses all future incomplete-struct warnings in the test file just to allow one suite initialization. Prefer constructing `WeightsTestSuite` as a zero-value and assigning `TestSuite`, so unrelated exhaustruct regressions still surface.
(Based on your team's feedback about preferring zero-value struct declarations over file-wide exhaustruct suppressions.) [FEEDBACK_USED]</violation>
</file>
<file name="x/emissions/module/rewards/rewards_multilabel_test.go">
<violation number="1" location="x/emissions/module/rewards/rewards_multilabel_test.go:343">
P2: Resetting the suite here changes the worker/reputer identities, so this test no longer isolates permutation invariance.</violation>
</file>
<file name="x/emissions/testutil/test_utils.go">
<violation number="1" location="x/emissions/testutil/test_utils.go:1">
P3: This file-level `exhaustruct` suppression is broader than necessary and will mask future partially initialized struct literals in `test_utils.go`; remove it and fix the specific zero-value literals locally instead.
(Based on your team's feedback about using zero-value `var` declarations instead of broad `exhaustruct` suppressions.) [FEEDBACK_USED]</violation>
</file>
<file name="x/emissions/keeper/inference_values_test.go">
<violation number="1" location="x/emissions/keeper/inference_values_test.go:1">
P3: Avoid a file-wide `exhaustruct` suppression here; it masks omitted-field mistakes for every struct literal in this test file.
(Based on your team's feedback about avoiding broad `//nolint` directives when zero-value construction or narrower fixes are available.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
xmariachi
reviewed
Mar 17, 2026
| s.Require().NotEmpty(reg.GetLabels()) | ||
|
|
||
| // Create and insert inferences (Values length must match registry length) | ||
| //nolint:exhaustruct |
Contributor
There was a problem hiding this comment.
The purpose of the exhaustruct linter is to make sure that code does not leave unassigned values so they are handled explicitly, preventing missing cases, improving code safety.
I think they should be filled in in easy cases like this (even if they are nil / empty).
xmariachi
requested changes
Mar 23, 2026
| } | ||
| } | ||
|
|
||
| mkExpectedCombined := func(args synth.GetCombinedInferenceArgs, weights synth.RegretInformedWeights) alloraMath.DecArray { |
Contributor
There was a problem hiding this comment.
This test replicates some logic that is on the actual code.
mkExpectedCombined is re-implementing the aggregation logic under test, which makes this test less independent: if the production logic and this helper share the same mistake, the test can still pass.
I’d suggest removing this helper and replacing these expectations with small hand-computed cases where possible, then keeping any unequal-regret coverage in a separate behavioral test (for example, asserting weight ordering and that the combined output shifts toward the forecast-implied values).
I have provided an implementation of this in #936 , please review.
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺ v ✰ Thanks for creating a PR! You're awesome! ✰ v Please note that maintainers will only review those PRs with a completed PR template. ☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --> ## Purpose of Changes and their Description Improve TestGetCombinedInference so it validates behavior more independently from the production aggregation logic. This updates the test to: replace helper-derived expected outputs with explicit expectations for simple scenarios keep direct checks for equal-regret and all-inferers-new behavior add a dedicated unequal-regret case to verify that forecaster-implied values shift the combined inference in the expected direction ### Why The previous test computed expected outputs with a helper that closely mirrored the implementation, which reduced its ability to catch logic regressions. These changes make the assertions easier to reason about while preserving coverage of weight-sensitive behavior. ## Are these changes tested and documented? - [ ] If tested, please describe how. If not, why tests are not needed. -- these code fixes tests. - [ ] If documented, please describe where. If not, describe why docs are not needed. -- no func changes , no need - [ ] Added to `Unreleased` section of `CHANGELOG.md`? -- no func changes , no need <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Strengthens `GetCombinedInference` unit tests by using independent, explicit expectations and adding weight-sensitive scenarios. Aligns with ENGN-6515 to reduce coupling to production logic and better catch regressions. - **Refactors** - Replaced helper-derived expectations with explicit values for simple cases. - Added cases for unequal-regret weighting and forecaster-implied inference (including distinct-regret forecaster). - Kept equal-regret mean and all-new-inferers averaging with direct `wantCombined` assertions. - Switched to `testutil2.InEpsilon5` for tolerant value comparisons and added targeted weight assertions. <sup>Written for commit 56e46fa. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. -->
There was a problem hiding this comment.
1 issue found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="test/integration/create_topic_test.go">
<violation number="1" location="test/integration/create_topic_test.go:105">
P2: `CreateTopicMultiLabel` substantially duplicates `CreateTopic`; extract shared topic-creation/assertion logic into one helper with configurable fields to avoid drift between the two paths.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| } | ||
|
|
||
| // test that we can create multi-label topics and that the resultant topics are what we asked for | ||
| func CreateTopicMultiLabel(m testCommon.TestConfig) (topicId uint64) { |
There was a problem hiding this comment.
P2: CreateTopicMultiLabel substantially duplicates CreateTopic; extract shared topic-creation/assertion logic into one helper with configurable fields to avoid drift between the two paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/integration/create_topic_test.go, line 105:
<comment>`CreateTopicMultiLabel` substantially duplicates `CreateTopic`; extract shared topic-creation/assertion logic into one helper with configurable fields to avoid drift between the two paths.</comment>
<file context>
@@ -100,3 +100,79 @@ func CreateTopic(m testCommon.TestConfig) (topicId uint64) {
}
+
+// test that we can create multi-label topics and that the resultant topics are what we asked for
+func CreateTopicMultiLabel(m testCommon.TestConfig) (topicId uint64) {
+ ctx := context.Background()
+ topicIdStart, err := m.Client.QueryEmissions().GetNextTopicId(
</file context>
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
2 participants
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.
Purpose of Changes and their Description
Add unit tests for multi-label support
Link(s) to Ticket(s) or Issue(s) resolved by this PR
Are these changes tested and documented?
Unreleasedsection ofCHANGELOG.md?Still Left Todo
Fill this out if this is a Draft PR so others can help.
Summary by cubic
Adds unit and integration tests for multi-label inference synthesis, rewards, and topic creation, plus test util refactors for labeled inputs and account reuse. Simplifies forecast‑implied APIs and strengthens classification #7 coverage with independent expectations and label‑space checks.
New Features
ConvertLabeledValuesToDecArray.GetWorkerMultiValuesFromIndexes,WithAccounts,FundAndWhitelistAccounts.Bug Fixes
DecMatrixJSON marshalling and usecopy()when building event matrices.Keeper.RegisterEpochLabel.unity_tolerancevalidation to reject negatives only whenRequireUnityis true.emissions/api/emissions/v10.Written for commit c5ab1b4. Summary will update on new commits.