Skip to content

feat(miner): route queue next through WIP-cap-aware claimer (#4850) - #5556

Closed
RealDiligent wants to merge 4 commits into
JSONbored:mainfrom
RealDiligent:feat/queue-next-wip-cap-4850
Closed

feat(miner): route queue next through WIP-cap-aware claimer (#4850)#5556
RealDiligent wants to merge 4 commits into
JSONbored:mainfrom
RealDiligent:feat/queue-next-wip-cap-4850

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Summary

  • Route queue next through PortfolioQueueManager.claimNextBatch() with configurable WIP caps.
  • Add resolvePortfolioQueueCaps() reading portfolioQueue.globalWipCap / perRepoWipCap from operator .gittensory-miner.yml, with env and CLI overrides.
  • Rebased onto merged feat(miner): respect --json on CLI error paths #5543 (--json error paths) and upstream --dry-run queue flags.

Closes #4850

Test plan

  • Cap resolution tests (config, env, CLI precedence)
  • queue next stops claiming once WIP cap is reached
  • Dry-run + JSON error paths covered
  • Typecheck and unit tests pass locally

Made with Cursor

@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 13, 2026 05:03
@superagent-security

Copy link
Copy Markdown
Contributor

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

RealDiligent and others added 3 commits July 13, 2026 13:06
…d#4850)

Wire queue next and claim-batch caps from .gittensory-miner.yml and env, so repeated queue next stops once global/per-repo WIP limits are reached.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.92%. Comparing base (de9e07a) to head (0cd7004).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5556   +/-   ##
=======================================
  Coverage   94.91%   94.92%           
=======================================
  Files         570      571    +1     
  Lines       45325    45356   +31     
  Branches    14675    14675           
=======================================
+ Hits        43020    43052   +32     
+ Misses       1571     1570    -1     
  Partials      734      734           
Flag Coverage Δ
shard-1 44.11% <0.00%> (-0.04%) ⬇️
shard-2 35.91% <84.61%> (+0.18%) ⬆️
shard-3 31.93% <0.00%> (-0.17%) ⬇️
shard-4 31.87% <0.00%> (+0.71%) ⬆️
shard-5 32.76% <41.53%> (-0.13%) ⬇️
shard-6 44.41% <80.00%> (+0.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/gittensory-miner/lib/cli.js 100.00% <ø> (ø)
...kages/gittensory-miner/lib/portfolio-queue-caps.js 100.00% <100.00%> (ø)
...ckages/gittensory-miner/lib/portfolio-queue-cli.js 98.11% <100.00%> (+0.51%) ⬆️
...es/gittensory-miner/lib/portfolio-queue-manager.js 92.30% <ø> (ø)
🚀 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 13, 2026
@loopover-orb

loopover-orb Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-13 05:13:36 UTC

10 files · 2 AI reviewers · 1 blocker · readiness 100/100 · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This PR routes `queue next` through the caps-aware `PortfolioQueueManager.claimNextBatch()` and adds `resolvePortfolioQueueCaps()` for config/env/CLI cap resolution, closing #4850. The batch-claim wiring and CLI dry-run/JSON error paths are well tested, but the env-var cap resolution has an asymmetric merge bug relative to the CLI-flag merge path that will silently zero out an unset cap.

Blockers

  • packages/gittensory-miner/lib/portfolio-queue-caps.js:32-39 (`readEnvCaps`) + :52-53 (`resolvePortfolioQueueCaps`): setting only ONE env var (e.g. `GITTENSORY_MINER_GLOBAL_WIP_CAP=5` without `GITTENSORY_MINER_PER_REPO_WIP_CAP`) replaces `caps` wholesale with `envCaps`, and `normalizePortfolioCaps` defaults any missing field to `0` (see portfolio-queue-manager.js `normalizePortfolioCaps`), so the per-repo cap silently becomes 0 instead of falling back to the config-file/default value — this zeroes WIP capacity for every repo and claiming stops entirely, unlike the CLI-flag merge a few lines below which correctly does `cliCaps.perRepoWipCap ?? caps.perRepoWipCap`; the env path needs the same `??` merge against the prior `caps` value.
  • packages/gittensory-miner/lib/portfolio-queue-caps.js:25 and packages/gittensory-miner/lib/portfolio-queue-caps.js:40 collapse partial config/env overrides through `normalizePortfolioCaps()`, so a real operator config like `portfolioQueue: { globalWipCap: 4 }` or env with only `GITTENSORY_MINER_GLOBAL_WIP_CAP=4` resolves `perRepoWipCap` to 0 and `queue next` cannot claim any repo; merge each layer over the prior caps instead, e.g. `const configCaps = readConfigCaps(...); let caps = normalizePortfolioCaps({ globalWipCap: configCaps?.globalWipCap ?? 1, perRepoWipCap: configCaps?.perRepoWipCap ?? 1 }); const envCaps = readEnvCaps(env); if (envCaps) caps = normalizePortfolioCaps({ globalWipCap: envCaps.globalWipCap ?? caps.globalWipCap, perRepoWipCap: envCaps.perRepoWipCap ?? caps.perRepoWipCap });`.
Nits — 5 non-blocking
  • packages/gittensory-miner/lib/portfolio-queue-caps.js:26-30 (`readConfigCaps`): the catch-all swallows any error (including fs read failures, not just YAML/JSON parse errors) and silently falls back to defaults — consider at least distinguishing 'file exists but unreadable' from 'malformed content'.
  • External brief flags `packages/gittensory-miner/lib/portfolio-queue-cli.js` at ~407 lines (over the 400-line convention) and `parsePortfolioQueueCapArgs` at cyclomatic complexity 11 (threshold 10) — worth a follow-up split, not blocking here.
  • packages/gittensory-miner/lib/portfolio-queue-caps.js:35 nesting depth 5 (ternary inside try inside function) — could extract the JSON/YAML branch into a small helper for readability.
  • test/unit/miner-portfolio-queue-caps.test.ts covers combined-env-vars-override but not a single-env-var-set scenario, which is exactly the case that would have caught the blocker above — add that case once the merge is fixed.
  • Fix `readEnvCaps`/`resolvePortfolioQueueCaps` to merge partial env overrides against the already-resolved config caps the same way CLI overrides do, then add a unit test for 'only one env var set'.

Why this is blocked

  • packages/gittensory-miner/lib/portfolio-queue-caps.js:32-39 (`readEnvCaps`) + :52-53 (`resolvePortfolioQueueCaps`): setting only ONE env var (e.g. `GITTENSORY_MINER_GLOBAL_WIP_CAP=5` without `GITTENSORY_MINER_PER_REPO_WIP_CAP`) replaces `caps` wholesale with `envCaps`, and `normalizePortfolioCaps` defaults any missing field to `0` (see portfolio-queue-manager.js `normalizePortfolioCaps`), so the per-repo cap silently becomes 0 instead of falling back to the config-file/default value — this zeroes WIP capacity for every repo and claiming stops entirely, unlike the CLI-flag merge a few lines below which correctly does `cliCaps.perRepoWipCap ?? caps.perRepoWipCap`; the env path needs the same `??` merge against the prior `caps` value.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. packages/gittensory-miner/lib/portfolio-queue-caps.js:32-39 \(\`readEnvCaps\`\) \+ :52-53 \(\`resolvePortfolioQueueCaps\`\): setting only ONE env var \(e.g. \`GITTENSORY\_MINER\_GLOBAL\_WIP\_CAP=5\` without \`GITTENSORY\_MINER\_PER\_REPO\_WIP\_CAP\`\) replaces \`caps\` wholesale with \`envCaps\`, and \`normalizePortfolioCaps\` defaults any missing field to \`0\` \(see portfolio-queue-manager.js \`normalizePortfolioCaps\`\), so the per-repo cap silently becomes 0 instead of falling back to the config-file/default value — this zeroes WIP capacity for every repo and claiming stops entirely, unlike the CLI-flag merge a few lines below which correctly does \`cliCaps.perRepoWipCap ?? caps.perRepoWipCap\`; the env path needs the same \`??\` merge against the prior \`caps\` value.
Signal Result Evidence
Code review ❌ 1 blocker 2 reviewers, synthesized
Linked issue ✅ Linked #4850
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 310 registered-repo PR(s), 142 merged, 25 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 310 PR(s), 25 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — Routing `queue next` through the existing WIP-cap-aware claimer is a meaningful operator-facing improvement for the linked issue, once partial cap precedence is fixed.
Linked issue satisfaction

Addressed
queue next now routes through PortfolioQueueManager.claimNextBatch() instead of the naive dequeueNext(), and a new resolvePortfolioQueueCaps() reads the WIP cap from .gittensory-miner.yml (with env/CLI overrides), matching the acceptance criterion that repeated queue next calls stop once the cap is reached.

Review context
  • Author: RealDiligent
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, JavaScript, Ruby, Svelte, TypeScript, Markdown, MDX
  • Official Gittensor activity: 310 PR(s), 25 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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

@loopover-orb

loopover-orb Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (AI reviewers agree on a likely critical defect: packages/gittensory-miner/lib/portfolio-queue-caps.js:32-39 (`readEnvCaps`) + :52-53 (`resolvePortfolioQueueCaps`): setting only ONE env var (e.g. `GITTENSORY_MINER_GLOBAL_WIP_CAP=5` without `GITTENSORY_MINER_PER_REPO_WIP_CAP`) replaces `caps` wholesale with `envCaps`, and `normalizePortfolioCaps` defaults any missing field to `0` (see portfolio-queue-manager.js `normalizePortfolioCaps`), so the per-repo cap silently becomes 0 instead of falling back to the config-file/default value — this zeroes WIP capacity for every repo and claiming stops entirely, unlike the CLI-flag merge a few lines below which correctly does `cliCaps.perRepoWipCap ?? caps.perRepoWipCap`; the env path needs the same `??` merge against the prior `caps` value.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 13, 2026
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wire WIP-cap-aware batch claiming into the real CLI path

1 participant