Skip to content

fix: resolve variable shadowing in score_organic_jobs that made score cap a no-op#847

Open
Grizouforever wants to merge 1 commit intobackend-developers-ltd:masterfrom
Grizouforever:fix/organic-job-score-cap-variable-shadowing
Open

fix: resolve variable shadowing in score_organic_jobs that made score cap a no-op#847
Grizouforever wants to merge 1 commit intobackend-developers-ltd:masterfrom
Grizouforever:fix/organic-job-score-cap-variable-shadowing

Conversation

@Grizouforever
Copy link
Copy Markdown

Summary

Fixes #846.

In score_organic_jobs, the loop variable score in for hotkey, score in batch_scores.items() shadowed the outer score = get_config("DYNAMIC_ORGANIC_JOB_SCORE") binding. This caused min(score, limit * score) to always equal score (because score was already the accumulated total, and limit >= 1), so DYNAMIC_SCORE_ORGANIC_JOBS_LIMIT was effectively ignored.

Changes

  • validator/app/src/compute_horde_validator/validator/scoring/calculations.py

    • Rename scoreper_job_score for the config value
    • Pre-compute cap = per_job_score * limit outside the loop
    • Use accumulated as the loop variable to eliminate the shadowing
  • validator/app/src/compute_horde_validator/validator/scoring/tests/test_score_organic_jobs.py (new)

    • 12 unit tests covering:
      • No-cap path (limit < 0): empty list, single job, multiple jobs, multiple miners, non-unit score
      • Capped path (limit >= 0): cap applied when exceeding limit, not applied when below, exact limit, non-unit per-job score, zero limit, per-miner independence
      • Regression test that would have caught the original bug

Test plan

  • All 12 new unit tests pass (pytest compute_horde_validator/validator/scoring/tests/test_score_organic_jobs.py)
  • No existing tests broken

… cap a no-op

In `score_organic_jobs`, the loop variable `score` in
`for hotkey, score in batch_scores.items()` shadowed the outer
`score = get_config("DYNAMIC_ORGANIC_JOB_SCORE")` binding.
This caused `min(score, limit * score)` to always equal `score`
(since `limit >= 1` in any meaningful config), so the
`DYNAMIC_SCORE_ORGANIC_JOBS_LIMIT` cap was never applied.

Fix: rename the outer variable to `per_job_score`, compute the cap
once as `cap = per_job_score * limit`, and use `accumulated` as the
loop variable so the names are unambiguous and the cap works correctly.

Also adds 12 unit tests covering the no-cap and capped paths,
including a regression test that would have caught the original bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: variable shadowing in score_organic_jobs makes DYNAMIC_SCORE_ORGANIC_JOBS_LIMIT cap a no-op

1 participant