Skip to content

test(miner): add root-level vitest coverage for AmsPolicySpec's parser - #5253

Merged
JSONbored merged 1 commit into
mainfrom
fix/ams-policy-spec-coverage-5132
Jul 12, 2026
Merged

test(miner): add root-level vitest coverage for AmsPolicySpec's parser#5253
JSONbored merged 1 commit into
mainfrom
fix/ams-policy-spec-coverage-5132

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

Advances #5132

#5249 (`.gittensory-ams.yml` operator execution-policy config) merged with `AmsPolicySpec` at 40.98% patch coverage -- a real codecov gap, not a false negative. Codecov only sees `packages/gittensory-engine` files through a root `test/unit/*.test.ts` that actually imports and exercises them; the engine package's own `node:test` suite (`packages/gittensory-engine/test/ams-policy-spec-parser.test.ts`, which already had 9/9 passing) is a completely separate, Codecov-invisible measurement.

`test/unit/miner-ams-policy.test.ts` only exercised `resolveAmsPolicy`'s own call path, leaving most of `parseAmsPolicySpec`'s field-level branches (submissionMode/slopThreshold rejection, capLimits/convergenceThresholds validation, malformed/oversized content, multi-byte UTF-8 byte-length counting) with zero Codecov-visible coverage.

Adds a root-level mirror of the engine's own parser test, matching the established `test/unit/miner-goal-spec-parser.test.ts` pattern (a root vitest file importing directly from `packages/gittensory-engine/src/index`, which vitest's coverage.include picks up without needing a miner-lib wrapper).

Test plan

  • `npx vitest run test/unit/ams-policy-spec-parser.test.ts --coverage --coverage.include="packages/gittensory-engine/src/ams-policy-spec.ts"` -- 100% statements/branches/functions/lines, confirmed locally against this exact branch's tree (was 40.98% before)
  • `npx vitest run test/unit/ams-policy-spec-parser.test.ts test/unit/miner-ams-policy.test.ts test/contract/engine-parity.test.ts` -- 35/35 passing
  • `npx tsc --noEmit`

Advances #5132

#5249 merged with AmsPolicySpec at 40.98% patch coverage: Codecov only
sees packages/gittensory-engine files through a root test/unit/*.test.ts
that actually imports and exercises them -- the engine package's own
node:test suite is a completely separate, Codecov-invisible
measurement. test/unit/miner-ams-policy.test.ts only exercised
resolveAmsPolicy's own call path, leaving most of parseAmsPolicySpec's
field-level branches (submissionMode/slopThreshold rejection,
capLimits/convergenceThresholds validation, malformed/oversized
content) uncovered from Codecov's perspective. Adds a root-level
mirror of the engine's own parser test, matching the established
miner-goal-spec-parser.test.ts pattern -- 100% statement/branch/line
coverage on ams-policy-spec.ts confirmed locally.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.36%. Comparing base (a6d93c6) to head (8630160).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5253      +/-   ##
==========================================
+ Coverage   94.27%   94.36%   +0.08%     
==========================================
  Files         474      474              
  Lines       40078    40078              
  Branches    14612    14612              
==========================================
+ Hits        37782    37818      +36     
+ Misses       1609     1585      -24     
+ Partials      687      675      -12     
Flag Coverage Δ
shard-1 46.24% <ø> (-0.15%) ⬇️
shard-2 34.73% <ø> (+0.15%) ⬆️
shard-3 32.08% <ø> (+0.03%) ⬆️
shard-4 31.85% <ø> (-0.28%) ⬇️
shard-5 33.69% <ø> (-0.03%) ⬇️
shard-6 44.99% <ø> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 12, 2026
@loopover-orb

loopover-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-12 11:52:48 UTC

1 file · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · unknown

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This is a pure test-addition PR that mirrors the existing engine-level node:test suite for parseAmsPolicySpec/parseAmsPolicySpecContent as a root-level vitest file, so Codecov (which only sees packages/gittensory-engine through root test/unit/*.test.ts) can actually measure the parser's branches. The new file imports directly from the engine barrel, exercises submissionMode/slopThreshold acceptance and rejection, capLimits/convergenceThresholds normalization and validation, and content-parsing edge cases (empty, malformed JSON/YAML, oversized, multi-byte UTF-8), matching the established miner-goal-spec-parser.test.ts pattern. All assertions match the parallel engine-package test file byte-for-byte in intent, CI is fully green including codecov/patch, and there is no production code touched.

Nits — 5 non-blocking
  • The multi-byte UTF-8 comment in the last `it` block explains intent well, but the test itself doesn't assert on `warnings` for the successful multi-byte parse, so a regression in utf8ByteLength counting that still stayed under the limit wouldn't be caught beyond the boundary case already covered by `oversized`.
  • Consider adding one boundary-case assertion where the multi-byte content sits right at the byte-length limit (rather than 'well under'), since that's the branch most likely to have an off-by-one bug in a byte-counting implementation.
  • nit: test/unit/ams-policy-spec-parser.test.ts:91 checks that non-finite capLimits.turns falls back, but it does not assert the warning text, so a regression that silently drops the invalid value would still pass.
  • nit: test/unit/ams-policy-spec-parser.test.ts:121 only checks a fully valid convergenceThresholds object, but the claimed independent normalization would be stronger if it also covered an invalid maxReenqueues value, not just maxConsecutiveFailures at line 124.
  • test/unit/ams-policy-spec-parser.test.ts:91: add an assertion like expect(nonFinite.warnings.join(" ")).toMatch(/capLimits\.turns/i) so the invalid-field diagnostic remains covered.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 44 registered-repo PR(s), 36 merged, 470 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 44 PR(s), 470 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ⚠️ ℹ️ Insufficient signal risk: clean · value: insufficient-signal — Nothing measurable for the structural-improvement analyzers on this PR (e.g. no code files changed).
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 44 PR(s), 470 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@JSONbored JSONbored self-assigned this Jul 12, 2026
@JSONbored
JSONbored merged commit 73a0912 into main Jul 12, 2026
18 checks passed
@JSONbored
JSONbored deleted the fix/ams-policy-spec-coverage-5132 branch July 12, 2026 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Development

Successfully merging this pull request may close these issues.

1 participant