Skip to content

tests: miner.py — first coverage (validate_local_config + sha256_dir) - #6

Merged
roykollensvendsen merged 3 commits into
tests/foundationfrom
tests/pr6-miner-validation
May 3, 2026
Merged

tests: miner.py — first coverage (validate_local_config + sha256_dir)#6
roykollensvendsen merged 3 commits into
tests/foundationfrom
tests/pr6-miner-validation

Conversation

@roykollensvendsen

Copy link
Copy Markdown
Owner

Summary

First test coverage for miner.py (previously 0% covered). Two pure
filesystem functions tested with tmp_path — no mocking needed.

Functions covered

Function Tests Spec source Outcome
validate_local_config(king_dir, challenger_dir) 7 docstring + sig + parallel inference from validator.validate_challenger_config confirms-behavior + 1 resolved spec-ambiguity
sha256_dir(path) 5 sig + name + call sites + validator comment ref confirms-behavior

12 tests, all green.

Coverage

validate_local_config — the miner-side equivalent of
validator.validate_challenger_config:

  • Matching config + safetensors → returns None
  • Missing king config → returns None (initial seed scenario)
  • Arch mismatch → rejected
  • Vocab mismatch → rejected
  • Missing challenger config → rejected
  • Missing safetensors → rejected
  • .py file present → rejected (security)

sha256_dir — content hash of *.safetensors files. Critical
because the miner encodes this hash on-chain and the validator
recomputes it; any divergence silently invalidates every miner's
commit:

  • Returns 64-char hex digest
  • Deterministic
  • Content-sensitive
  • Non-safetensors files ignored
  • Sort-invariant across filesystem ordering

Findings during testing

  • Spec-ambiguity resolved: my initial happy-path test for
    validate_local_config only set up config.json. It went RED with
    "no .safetensors files in challenger" — the function also checks
    the directory contents. Reframed the helper (_make_valid_challenger)
    to include safetensors and added an explicit
    test_..._missing_safetensors_rejects test to lock in the
    discovered behavior. Also added test_..._python_file_rejects since
    the same security rule from validate_challenger_config applies.

Verification

.venv/bin/python -m pytest tests/test_miner_validation.py -v
# 12 passed
ruff check --config ../scripts/teutonic.ruff.toml tests/
# All checks passed!

Series context

PR 6 of foundation series. With this merged, foundation will have
68 tests across 11 functions/surfaces. Coverage now spans
validator.py, eval_torch.py, AND miner.py (no longer 0%).

PR 5 (new helpers from upstream
unarbos/teutonic#3)
remains the only deferred item — those symbols don't exist in the
current main yet.

Spec source: docstring ("Compare king and challenger config.json
locally. Returns None if OK, or a human-readable rejection reason.")
+ signature + parallel inference from validator.validate_challenger_config.

Outcome: confirms-behavior plus one resolved spec-ambiguity. 7 tests
cover the happy path, missing king config (returns None per docstring),
arch and vocab mismatches, missing challenger config, missing
safetensors, and *.py upload rejection.

Spec-ambiguity resolved: my initial happy-path test only set up
config.json on both sides. It went RED because the function also
requires .safetensors files in the challenger directory. Added a
helper (_make_valid_challenger) and an explicit
test_..._missing_safetensors_rejects test to lock in the discovered
spec.

Also added test_..._python_file_rejects since the security rule from
validate_challenger_config (deny *.py to prevent auto_map code execution)
should reasonably apply on the miner side too — confirmed it does.

This is the first coverage for miner.py (previously 0% covered).
Spec source: signature (no docstring) + name + call sites in miner.py
(king_hash = sha256_dir(king_dir), challenger_hash = sha256_dir(challenger_dir))
+ comment in validator.py:776 ("king_hash miners encode in their
on-chain commits — see miner.sha256_dir").

Outcome: confirms-behavior. 5 tests cover return format (64-char hex),
determinism, content-sensitivity, that non-.safetensors files are
ignored, and that multiple .safetensors files combine deterministically
regardless of filesystem ordering (the impl sorts).

The sort-invariance test is the highest-value: miner and validator
must agree on the king hash regardless of which order files were
written to disk; a bug there would silently invalidate every miner's
on-chain hash commit.
Two additional tests covering edge cases I identified while reviewing
the PR diff:

- `test_validate_local_config_accepts_sharded_safetensors`: real-world
  models are typically sharded into multiple safetensors files
  (model-0000N-of-N.safetensors). Confirms multi-file checkpoints are
  accepted, not just single-file ones.
- `test_sha256_dir_empty_directory_returns_empty_hash`: documents the
  defined behavior for an empty directory (returns hashlib.sha256()
  empty digest e3b0c44...).
@roykollensvendsen

Copy link
Copy Markdown
Owner Author

Self-review

Substantive (added in a951923)

  • Multi-shard happy path for validate_local_config. Original
    happy-path test used a single model.safetensors. Real-world
    checkpoints are typically sharded (model-00001-of-N.safetensors).
    Added test confirms multi-file is accepted — catches a hypothetical
    regression where someone tightened the check to "exactly one file".

  • Empty-directory test for sha256_dir. Documents defined
    behavior for the edge case (no .safetensors files): returns
    hashlib.sha256().hexdigest(). Real scenario: state cleared but
    directory not yet removed. Catches accidental crash-on-empty.

Verified

  • All 14 tests passing locally + ruff clean
  • Coverage spans both miner-side validation paths (config compare +
    content hash)
  • TDD discovery preserved in commit history: 643c879 (initial,
    spec-ambiguity finding) + 904af2c (sha256_dir) + a951923
    (self-review edge cases)

Considered, not changed

  • Subdirectory test for sha256_dir — would catch an accidental
    switch from glob to rglob. The sort-invariance test partially
    locks in the file-enumeration semantics. Marginal additional value.
  • .pyc / hidden file pattern variations for the .py rejection
    rule. The impl probably uses a simple suffix check; testing edge
    cases would lock in arbitrary impl details.
  • Performance/large-file behavior for sha256_dir. Correctness is
    the test target; perf is a different concern.

Series context

PR 6 of foundation series. With this merged, foundation will have
70 tests across 11 functions/surfaces. miner.py goes from 0% to
covered for both pure helpers; main() entry point remains untested
(intentionally — entry points typically aren't unit-tested).

@roykollensvendsen
roykollensvendsen merged commit 23a8e4c into tests/foundation May 3, 2026
@roykollensvendsen
roykollensvendsen deleted the tests/pr6-miner-validation branch May 3, 2026 11:15
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.

1 participant