Skip to content

refactor: bundle of small refactors - #917

Merged
anderdc merged 11 commits into
entrius:testfrom
ebios-star:refactor/use-base-github-api-url-constant
May 6, 2026
Merged

refactor: bundle of small refactors#917
anderdc merged 11 commits into
entrius:testfrom
ebios-star:refactor/use-base-github-api-url-constant

Conversation

@ebios-star

@ebios-star ebios-star commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Per @anderdc's review, consolidating my open small refactor PRs into this one. #864 and #873 are mid-review and remain separate as previously requested. (Note: #873 has since merged — its work is built on by the last commit here.)

Each commit is preserved as a separate revision so the bundle can be reviewed commit-by-commit or squash-merged.

Commits in this bundle (oldest → newest)

  1. refactor: bundle of small refactors #917refactor: use BASE_GITHUB_API_URL constant in cli/issue_commands/helpers.py
  2. refactor: use GITHUB_HTTP_TIMEOUT_SECONDS at the three remaining timeout=15 sites #942refactor: use GITHUB_HTTP_TIMEOUT_SECONDS at the three remaining timeout=15 sites
  3. refactor: remove unused REPO_SCAN_* constants #943refactor: remove unused REPO_SCAN_* constants
  4. refactor: inline single-use get_github_pat and is_comment_node helpers #948refactor: inline single-use get_github_pat and is_comment_node helpers
  5. refactor: inline single-use _calculate_base_score adapter #949refactor: inline single-use _calculate_base_score adapter into score_mirror_pr
  6. refactor: centralize ink! issues-mapping selector as ISSUES_MAPPING_ROOT_KEY #952refactor: centralize ink! issues-mapping selector as ISSUES_MAPPING_ROOT_KEY
  7. refactor: remove unused U32_MAX constant #953refactor: remove unused U32_MAX constant
  8. refactor: remove unused GITHUB_DOMAIN constant #954refactor: remove unused GITHUB_DOMAIN constant
  9. refactor: bundle of small refactors #955refactor: normalize __version__ import in neurons/validator.py
  10. refactor: move MAX_ISSUE_ID to gittensor/constants.py and use it on the validator side #1015refactor: move MAX_ISSUE_ID to gittensor/constants.py and use it on the validator side (follow-up to merged refactor: dedupe the 1_000_000 issue-ID bound in cli/issue_commands/helpers.py #873)

Aggregate change

  • 10 files changed, +28 / -52 lines (net -24 lines).
  • No behaviour change in any commit.

Type of Change

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

Testing

  • pytest tests/ on the merged bundle — all 754 tests pass.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Changes are documented (if applicable)

@ebios-star

Copy link
Copy Markdown
Contributor Author

Hi @anderdc — friendly review request whenever you have a moment. 3-line diff, no tests added, no new comments. Same shape as the merged #455.

@xiao-xiao-mao xiao-xiao-mao Bot added the refactor Code restructuring without behavior change label Apr 30, 2026

@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.

Please consolidate your open refactor PRs into this one: #942, #943, #948, #949, #952, #953, #954, #955. Easier to review as a single bundle. #864 and #873 are already mid-review — leave those separate.

anderdc
anderdc previously approved these changes May 5, 2026
@anderdc

anderdc commented May 5, 2026

Copy link
Copy Markdown
Collaborator

still need to consolidate your PRs here

@anderdc

anderdc commented May 6, 2026

Copy link
Copy Markdown
Collaborator

awaiting consolidation still, or I will close this PR

ebios-star added 10 commits May 6, 2026 17:05
…ers.py

Two `validate_repository` / `validate_github_issue` requests hardcoded
`https://api.github.com/...` instead of reusing the existing
`gittensor.constants.BASE_GITHUB_API_URL`. The file already imports
from `gittensor.constants` (NETWORK_MAP); just add the existing
constant alongside.

Same shape as entrius#455 (use shared NETWORK_CHOICE constant).
…out=15 sites

`gittensor/utils/github_api_tools.py` already imports
`GITHUB_HTTP_TIMEOUT_SECONDS` from `gittensor.constants` and uses it for
the `/user` request. Three other GitHub HTTP calls in the same file
(`get_merge_base_sha`, `get_pull_request_file_changes`,
`check_github_issue_closed`) still passed the literal `15`. Switch them
to the constant so a future timeout tweak doesn't need to be applied
in four places.
These three constants were only referenced by gittensor/validator/issue_discovery/repo_scan.py, which was deleted in entrius#796 when issue discovery was routed through das-github-mirror. Verified zero remaining callers across the codebase.
Two private 3-line wrappers each have exactly one caller in their own
module:

- get_github_pat (cli/issue_commands/helpers.py) wraps a single
  os.environ.get; the caller already does '... or ""', so the wrapper's
  'or None' fallback is collapsed away on inline.
- is_comment_node (validator/utils/tree_sitter_scoring.py) wraps a
  single 'node.type in COMMENT_NODE_TYPES' membership check.

grep confirms no other call sites or test references in gittensor/,
neurons/, or tests/.
…mirror_pr

The private `_calculate_base_score` is described in its own docstring as a
'Thin wrapper: run the shared helper and copy fields onto ScoredMirrorPR'
and has exactly one caller. It mutates seven fields on `scored` while
returning only `base_score`, hiding the bulk of its side effects behind
a misleading return signature.

Inlining moves the field-copying out into the caller alongside the
`scored.base_score = ...` assignment that already lived there, so all
eight mutations are visible on one block. The shared helper
`calculate_base_score_for_pr_files` (which actually computes the
score) is unaffected and still callable from any future site that
wants a non-mutating result.

`grep -rn _calculate_base_score` confirms no other call sites or test
references in `gittensor/`, `neurons/`, or `tests/`.
…OOT_KEY

The ink! storage selector '52789899' for the issues mapping is duplicated
at the two active call sites that compute lazy keys for child storage
reads:

- validator/issue_competitions/contract_client.py (read_issue_from_child_storage)
- cli/issue_commands/helpers.py (_read_issues_from_child_storage)

Both already import from validator/issue_competitions/storage_utils, so
this hoists the literal into a single named constant in storage_utils
and updates both call sites to use it. If the contract storage layout
ever changes, the selector now lives in one place beside the other
storage-decoding helpers.
U32_MAX is defined in neurons/base/utils/weight_utils.py but has zero
references across gittensor/, neurons/, and tests/. The sibling U16_MAX
is actively used by convert_weights_and_uids_for_emit and
process_weights_for_netuid.

Verified via 'grep -rn U32_MAX' and 'grep -rn 4294967295' — both return
only the definition line.
GITHUB_DOMAIN is defined at module scope in gittensor/classes.py but
has zero references anywhere in gittensor/, neurons/, or tests/. The
sites that build GitHub URLs use inline f-strings (e.g.
f'https://github.com/{repo}/issues/{issue_number}') and don't import
this constant.

Verified via 'grep -rn GITHUB_DOMAIN' — only the definition line.
The other two consumers of __version__ (gittensor/cli/main.py and
tests/cli/test_miner_commands.py) write 'from gittensor import
__version__'. neurons/validator.py used the non-canonical
'from gittensor.__init__ import __version__' form, which Python
resolves identically but reads as if it were targeting an internal
implementation file rather than the package's public surface.
…he validator side

Follow-up to entrius#873 which deduped the 1_000_000 issue-ID bound on the CLI
side using the helpers.py-local MAX_ISSUE_ID. The validator-side path
in IssueCompetitionContractClient.get_issues_by_status still carried
its own local 'MAX_REASONABLE_ISSUE_ID = 1_000_000' with the same value
and same intent.

Hoists MAX_ISSUE_ID into gittensor/constants.py (alongside the other
issues-competition constants) so both consumers share one definition.
Validator code stays clean of any cli/* dependency, matching the
import-direction precedent in entrius#425 (NETWORK_MAP) and entrius#460
(get_contract_address).
@ebios-star
ebios-star force-pushed the refactor/use-base-github-api-url-constant branch from 4c0636f to 143c216 Compare May 6, 2026 21:11
@ebios-star ebios-star changed the title refactor: use BASE_GITHUB_API_URL constant in cli/issue_commands/helpers.py refactor: bundle of small refactors May 6, 2026
@ebios-star

Copy link
Copy Markdown
Contributor Author

@anderdc done — force-pushed this branch with the 10 commits consolidated (#917 plus #942, #943, #948, #949, #952, #953, #954, #955, #1015). Each commit is preserved individually so the bundle can be reviewed commit-by-commit or squash-merged. #864 still mid-review; #873 has since merged and the last commit here builds on it. Thanks for the review!

@anderdc
anderdc merged commit 4ae445e into entrius:test May 6, 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.

2 participants