fix(standings): register a miner only after a numeric score is confirmed - #465
Open
rsnetworkinginc wants to merge 3 commits into
Open
fix(standings): register a miner only after a numeric score is confirmed#465rsnetworkinginc wants to merge 3 commits into
rsnetworkinginc wants to merge 3 commits into
Conversation
compute_standings registered a miner via per_miner.setdefault before any numeric per-benchmark score was seen. A merged competition win whose per_benchmark is empty or entirely non-numeric therefore created a phantom MinerStanding(overall=0.0, n_competed=0, rank=1) that could be falsely reported as standings.leader, contradicting the module contract that an overall leader must be strong on every benchmark and diverging from the non-dict per_benchmark case, which already drops such records. Move the setdefault inside the numeric-score branch so a miner is registered only once a real score is confirmed; empty/all-non-numeric wins now contribute nothing, matching the non-dict case. Add a regression test asserting such a win creates no phantom miner and no false leader.
ruff is unpinned (>=0.5) in the dev extras, so CI picked up the new ruff 0.16.0 release, which flags 300+ pre-existing violations across src/ and scripts/ on main (unrelated to this change). Pin to <0.16, the newest series main passes cleanly, so the lint gate checks this PR against the same ruleset the rest of the tree was written for.
…e punctuation tests/test_drop_leading_decimal_point.py (merged with James-CUDA#423) and the hyphen/edge-punctuation tokenizer changes landed from separate branches and semantically collided: raw.strip() treats the . of ".5." or "$.5" as wrapping noise, so the token normalizes to 5.0 - equal to a gold 5 (false positive) and unequal to the value-identical 0.5 (false negative). Main currently fails those three tests, which keeps every PR red at the Pytest step. Stop the left-edge strip as soon as removing one more character would break a leading decimal (a . directly followed by a digit); trailing punctuation still strips as before. All drop/BBH scoring tests pass.
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.
Type
Only routing head submissions that improve accuracy earn TAO via Gittensor.
General improvement
What does this PR do?
Fixes a phantom-miner bug in
compute_standings(src/trinity/standings.py). A minerwas registered via
per_miner.setdefault(str(miner), {})before any numericper-benchmark score was confirmed. A merged competition win whose
per_benchmarkisempty (or entirely non-numeric) therefore created a phantom entry that later surfaced as
MinerStanding(overall=0.0, n_competed=0, rank=1)and could be falsely reported asstandings.leader— despite the miner never recording a single valid benchmark score.The
setdefaultis moved inside the numeric-score branch, so a miner is registeredonly once a real numeric score is seen. Empty / all-non-numeric wins now contribute
nothing, exactly matching the existing behavior for the non-dict
per_benchmarkcase(which already drops the record).
Why is it needed?
It contradicts the module's contract — "degrades to empty ranking; an overall leader
must be strong on every benchmark." A win with no confirmed score is not strength on any
benchmark, yet it could win the crown. It was also internally inconsistent: a non-dict
per_benchmarkwas dropped, but an empty dict was not. This makes the two cases behaveidentically and prevents a false leader from being reported.
A dedicated regression test (
test_win_with_no_numeric_score_creates_no_phantom_leaderin
tests/test_standings.py) asserts that (a) empty and all-non-numeric merged winsregister no miner and yield
leader is None, and (b) a phantom win never shadows a realscored miner. Verified RED→GREEN: reverting the source change makes the test fail with a
phantom
ghostat rank 1; restoring it passes.Checklist
pytest tests/(new regression test added; standings suite green)ruff check src/mypy src/(for changed files)