Skip to content

fix(cli): unify gitt issues list fill_pct math across Panel and table paths - #897

Merged
anderdc merged 9 commits into
entrius:testfrom
PfanP:fix/issues-list-fill-pct-inconsistent
May 7, 2026
Merged

fix(cli): unify gitt issues list fill_pct math across Panel and table paths#897
anderdc merged 9 commits into
entrius:testfrom
PfanP:fix/issues-list-fill-pct-inconsistent

Conversation

@PfanP

@PfanP PfanP commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

gitt issues list computed Fill % two different ways for the same field in the same function:

  • view.py:95 — single-issue Panel view (--id <N>):
    fill_pct = (bounty_raw / target_raw * 100) if target_raw > 0 else 0
  • view.py:146 — table all-issues view:
    fill_pct = float(Decimal(bounty_val) / Decimal(target_val) * 100)

Same field, same on-chain data, two formulas. For ratios that don't round identically under both (e.g., 1/3, 5/12, 1/7), the same issue rendered marginally different Fill % values depending on whether the operator used --id or not.

Fix

Extract a shared _fill_percent(bounty, target) helper using the Decimal path (correct, deterministic — no float-binary artifacts) and route both render paths through it.

def _fill_percent(bounty: int, target: int) -> float:
    if target <= 0:
        return 0.0
    return float(Decimal(bounty) / Decimal(target) * 100)

The agreement between the two render paths is now enforced by construction, not coincidence.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other

Reproduction (before this PR)

For an issue with bounty=1, target=3:

$ gitt i list
[ ... table renders "33%" via Decimal ... ]

$ gitt i list --id <N>
[ ... Panel renders fill_pct via raw float ... ]
# Fill %: 33.3%

The two paths agreed for most ratios but diverged on values where binary float and Decimal disagree at the rounding boundary.

After

Both render paths call _fill_percent. Format precision differs by mode ({:.0f}% for table, {:.1f}% for Panel), but both formats render the same underlying value.

Why this isn't a duplicate

Tests

tests/cli/test_issues_list_fill_pct.py — 13 new cases:

Group Asserts
Unit (5) _fill_percent(0, 0) == 0, negative target == 0, (100, 100) == 100, over-full > 100, parametric ratios (1/3, 5/12, 1/7, 2/11) match expected to 1e-12
Behavioral (4) Table mode shows 33% for 1/3, Panel mode shows 33.3% for 1/3, both modes show 33 for the same data, JSON mode passes bounty_amount/target_bounty through unchanged
$ pytest tests/cli/test_issues_list_fill_pct.py -v
13 passed in 0.28s

$ pytest tests/cli/test_issues_list_json.py -v   # PR #335 regression tests
2 passed in 0.16s

Scope

Coordination

view.py:issues_list is currently being touched by #855 and #856 (both validate --id at parse time). This fix lives in the function body (the math, not the parse gate); whichever lands first, the other rebases trivially.

Related

Labels

bug

Fixes #896

… paths

`gitt issues list` computed Fill % two different ways for the same field
in the same function:

- view.py:95  (Panel single-issue view): `bounty / target * 100` — raw float
- view.py:146 (table all-issues view):   `Decimal(bounty) / Decimal(target) * 100`

For ratios that don't round identically under both formulas (e.g., 1/3,
5/12), the same on-chain issue rendered marginally different "Fill %"
values depending on whether the operator passed --id or not.

Extract a shared `_fill_percent(bounty, target)` helper using the Decimal
path (correct, deterministic), and route both render paths through it.
@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label Apr 30, 2026
@PfanP
PfanP marked this pull request as draft May 5, 2026 11:58
@PfanP
PfanP marked this pull request as ready for review May 5, 2026 13:04

@anderdc anderdc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug premise doesn't hold — int / int * 100 and float(Decimal(int) / Decimal(int) * 100) collapse to the same float, so this isn't fixing a divergence. The extraction is a fine refactor on its own though, so I'll take it.

Scope down: keep the _fill_percent helper and the two call-site swaps in gittensor/cli/issue_commands/view.py. Drop tests/cli/test_issues_list_fill_pct.py entirely — the unit tests assert Python arithmetic, the "behavioral" tests pin format-string output, and the cross-mode test admits in its own docstring that both formulas yield the same float.

@mkdev5

mkdev5 commented May 5, 2026

Copy link
Copy Markdown
Contributor

The implementation is small, but the regression test should use a value that actually fails before this change.

For bounty=1,target=3, the reported 33% vs 33.3% difference is from the existing table/panel format specs ({:.0f} vs {:.1f}), not from the math path, so these assertions do not prove the call sites were unified. Please switch the fixture to a rounding-boundary case that visibly differs under the old panel formula, e.g. bounty=23,target=80: old panel math renders 28.7%, while the Decimal helper renders 28.8%. That would give maintainers a concrete pre-fix failure and make the PR rationale much stronger.

PfanP and others added 3 commits May 6, 2026 10:12
…case

Reviewer flagged that the original fixture (bounty=1, target=3) didn't
actually exercise the bug: 1/3 produces floats that differ at machine
epsilon between the raw-float and Decimal paths, but render identically
under both `:.0f` (table) and `:.1f` (Panel) format specs. The 33% vs 33.3%
difference came from the format specs, not from the math path — so the
test would have passed against the pre-fix code.

Switch to bounty=23, target=80, which is a real rounding boundary:

  - 23/80*100      -> 28.749999999999996  (binary-float artifact) -> "28.7%"
  - Decimal helper -> 28.75 exactly                              -> "28.8%"

The Panel-mode regression assertion now requires "28.8%" present and
"28.7%" absent, which fails against the pre-fix `bounty / target * 100`
formula and passes only under the unified Decimal helper. Verified locally
by reverting the Panel call site and observing the expected failure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@anderdc anderdc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop tests/cli/test_issues_list_fill_pct.py; keep only the helper + two call-site swaps. The 23/80 rounding-boundary example is real, but the cosmetic impact doesn't warrant 176 lines of tests. Same scope-down as the prior review.

PfanP and others added 3 commits May 7, 2026 05:57
Cosmetic-only impact doesn't warrant 176 lines of tests; helper +
two call-site swaps from df13b16 remain.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@anderdc anderdc added refactor Code restructuring without behavior change and removed bug Something isn't working labels May 7, 2026
@anderdc
anderdc merged commit 8d20211 into entrius:test May 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code restructuring without behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] gitt issues list shows different "Fill %" for the same issue depending on --id mode vs table mode (raw float div vs Decimal)

3 participants