Skip to content

fix(portfolio): scope per-repo WIP caps by forge host, not repo name alone (#7224) - #7261

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
xfodev:fix/portfolio-wip-caps-per-host-7224
Jul 19, 2026
Merged

fix(portfolio): scope per-repo WIP caps by forge host, not repo name alone (#7224)#7261
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
xfodev:fix/portfolio-wip-caps-per-host-7224

Conversation

@xfodev

@xfodev xfodev commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

The portfolio store's #5563 composite primary key (api_base_url, repo_full_name, identifier) keeps two
forge hosts serving a same-named owner/repo as distinct rows — but the two WIP-cap/selection paths built on
top of it dropped the host dimension, so a per-repo cap was silently shared across hosts:

  • entriesToPortfolioQueue (portfolio-queue-manager.js) folded active rows for the engine's nextEligibleItems,
    which groups its per-repo WIP cap by each item's repoFullName — an opaque string to the engine (it has no
    apiBaseUrl concept; the host is smuggled through the opaque id via queueItemId, Scope local ledger keys by forge host, not bare repoFullName #5563). Two hosts' rows
    therefore merged into one group, so e.g. perRepoWipCap: 1 let only ONE host's backlog advance.
  • selectNextEligibleTarget (portfolio-queue-cli.js, queue next --per-repo-wip) counted repoActiveCount
    by repoFullName alone, so an in-progress item on host A blocked a queued item for the same repo on host B.

This host-qualifies both:

  • Each item's engine-grouping repoFullName is now ${apiBaseUrl}\n${repo} — the same "smuggle the host through
    the string the engine treats opaquely" approach queueItemId already uses for the id. The item id still
    carries the true repo, and selectEligibleBatch maps results back via parseQueueItemId(id), so the real
    repo/host reach the caller unchanged. The returned bucket's own repoFullName stays the plain repo name.
  • selectNextEligibleTarget's repoActiveCount filter additionally matches entry.apiBaseUrl === topQueued.apiBaseUrl.

Single-host behavior (the overwhelming common case) is unchanged: one apiBaseUrl means one grouping key per
repo, and the added filter match is always true — confirmed by the existing suites passing untouched.

Closes #7224

Scope

  • Conventional Commit title; focused (two modules + their tests); no site//CNAME/lovable; no new dependency.
  • Linked a currently open issue (Closes #7224).

Validation

  • git diff --check
  • npm run typecheck (root — clean)
  • npm run test:coverage on both changed modules — 100% of the changed lines + branches covered. Two new
    regressions with a cap that actually binds (perRepoWipCap: 1, which the existing Scope local ledger keys by forge host, not bare repoFullName #5563 test at cap 2 could
    not catch): claimNextBatch claims both hosts' same-named rows, and selectNextEligibleTarget does not let
    host A's in-progress item block host B's queued one. All existing portfolio-queue tests pass unchanged.
  • npm run build:miner (node --check on both files passes).

If any required check was skipped, explain why:

  • Backend change confined to packages/loopover-miner/lib/**; CI runs the full suite.

Safety

  • No secrets, wallets, hotkeys, coldkeys, PATs, trust scores, or private data exposed.
  • Public GitHub text stays sanitized and low-noise.
  • No auth/CORS/session/API/OpenAPI/MCP surface change — internal WIP-cap accounting only; return shapes are unchanged.
  • No UI change — no UI Evidence needed.

@xfodev
xfodev requested a review from JSONbored as a code owner July 19, 2026 11:00
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.81%. Comparing base (a575ac2) to head (e445bc8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7261   +/-   ##
=======================================
  Coverage   93.81%   93.81%           
=======================================
  Files         704      704           
  Lines       69506    69508    +2     
  Branches    18912    18912           
=======================================
+ Hits        65210    65212    +2     
  Misses       3302     3302           
  Partials      994      994           
Flag Coverage Δ
shard-1 43.16% <85.71%> (-0.40%) ⬇️
shard-2 37.30% <14.28%> (+0.12%) ⬆️
shard-3 32.94% <0.00%> (-0.14%) ⬇️
shard-4 32.70% <0.00%> (-1.60%) ⬇️
shard-5 32.86% <85.71%> (+0.59%) ⬆️
shard-6 46.03% <85.71%> (+0.27%) ⬆️

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

Files with missing lines Coverage Δ
packages/loopover-miner/lib/portfolio-queue-cli.js 98.92% <100.00%> (ø)
...ages/loopover-miner/lib/portfolio-queue-manager.js 98.00% <100.00%> (+0.08%) ⬆️

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

loopover-orb Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-19 11:10:43 UTC

4 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This host-qualifies the two backlog-selection paths that previously ignored `apiBaseUrl`: `entriesToPortfolioQueue` now groups engine items by `${apiBaseUrl}\n${repoLower}` (bucket's own `repoFullName` stays the plain lowercased repo for display, while `selectEligibleBatch` still recovers the true repo/host via `parseQueueItemId(id)`), and `selectNextEligibleTarget`'s per-repo active-count filter now also matches `apiBaseUrl`. The manager-level fix is validated by a genuine end-to-end regression test that calls the real (non-mocked) `@​loopover/engine` `nextEligibleItems` via `claimNextBatch()` and asserts both hosts' same-named-repo items claim under a binding cap of 1, which is strong evidence the engine really groups its per-repo cap by the item's opaque `repoFullName` field as described. Scope is tight (two modules + their tests) and single-host behavior is provably unchanged since the added filter/grouping dimension collapses to one value.

Nits — 3 non-blocking
  • portfolio-queue-manager.js:73 — using `\n` as the apiBaseUrl/repo separator in `repoKey` is safe in practice but undocumented as collision-proof; a one-line comment noting apiBaseUrl can't itself contain `\n` would remove any doubt for the next reader.
  • The inline comments in both files are quite long (multi-sentence blocks) for what are fairly small logic changes — could trim to the core rationale and let the PR description carry the rest.
  • Consider extracting the `${apiBaseUrl}\n${repoLower}` construction into a small named helper (e.g. `hostScopedRepoKey`) shared between the grouping key and the item's `repoFullName`, so the two call sites can't drift if the separator ever changes.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7224
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: 19 registered-repo PR(s), 9 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor xfodev; Gittensor profile; 19 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The diff host-qualifies entriesToPortfolioQueue's bucket key via a compound apiBaseUrl+repoFullName key (mapped back to real values through queueItemId/parseQueueItemId) and adds an apiBaseUrl match to selectNextEligibleTarget's repoActiveCount filter, matching both stated requirements, and includes both requested regression tests plus preserves single-host behavior.

Review context
  • Author: xfodev
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, TypeScript
  • Official Gittensor activity: 19 PR(s), 0 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.
🧪 Chat with LoopOver

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

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

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 4cd3f24 into JSONbored:main Jul 19, 2026
16 checks passed
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.

fix(portfolio): scope per-repo WIP caps and diversification buckets by apiBaseUrl, not repoFullName alone

1 participant