Skip to content

fix(miner-ui): reunify the portfolio view with the CLI's own richer dashboard - #5479

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
galuis116:fix/miner-ui-portfolio-dashboard-reunify
Jul 12, 2026
Merged

fix(miner-ui): reunify the portfolio view with the CLI's own richer dashboard#5479
JSONbored merged 1 commit into
JSONbored:mainfrom
galuis116:fix/miner-ui-portfolio-dashboard-reunify

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Summary

  • The miner-ui's portfolio view re-implemented its own narrower, global-only aggregation ({ total, counts }) instead of reusing the CLI's own richer, per-repo queue dashboard output — a real functional regression, not just duplicated code, and the miner-ui and CLI maintained two separate data paths for the same underlying miner_portfolio_queue store.
  • apps/gittensory-miner-ui/vite-portfolio-queue-api.ts (the local dev-server API middleware) now calls packages/gittensory-miner/lib/portfolio-dashboard.js's existing collectPortfolioDashboard — the SAME pure aggregator gittensory-miner queue dashboard and the read-only gittensory_miner_get_portfolio_dashboard MCP tool already use — instead of a bespoke, narrower re-implementation. One shared data path now feeds the CLI, the MCP tool, and the UI.
  • apps/gittensory-miner-ui/src/lib/portfolio-queue.ts's PortfolioQueueSummary type now matches the CLI's dashboard shape ({ total, byStatus, repos: [{repoFullName, byStatus, total}], oldestQueuedAgeMs }) instead of the old flat { total, counts }.
  • apps/gittensory-miner-ui/src/routes/portfolio.tsx now renders a per-repo breakdown table (using the shared @jsonbored/gittensory-ui-kit Table component, matching the existing run-history.tsx route's pattern) beneath the existing global summary cards, so the view shows the same per-repo detail the CLI's queue dashboard already shows — not just a global aggregate.
  • The API still aggregates server-side and never republishes raw per-item queue identifiers or rank-derived priorities — only status counts, now grouped globally and per repo, cross the wire. Repo full names ARE now exposed (the CLI's own dashboard already prints them locally, so they were never treated as sensitive — only identifier/priority are).

Fixes #4846

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally — this change lives entirely under apps/gittensory-miner-ui/** (a separate vitest workspace covered by ui:test, not the root test:coverage/Codecov-measured slice) and packages/gittensory-miner/**, which sits outside vitest's coverage.include glob today.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run build:miner
  • npm run test:miner-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:test
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Ran the full apps/gittensory-miner-ui test suite (33/33 passing, including 14 in the rewritten portfolio-queue.test.tsx) plus npm run ui:lint/ui:typecheck/ui:test/ui:build at the root (all green). Also manually started the real dev server and curled GET /api/portfolio-queue against a fresh (no local state) install to confirm the live endpoint returns the new shape end-to-end ({"summary":{"total":0,"byStatus":{...},"repos":[],"oldestQueuedAgeMs":null}}), not just the unit-test doubles.

Test coverage added/updated: per-repo table rendering (row-per-repo, correct per-status columns), the fresh-install/loading/error states unchanged, the API handler's wiring to the (now dynamically-imported, to avoid a node:sqlite-via-portfolio-queue.js bundling failure in the UI's client/test environment) dashboard aggregator, the fresh-install fast path (never initializes the store), a 500-on-store-failure path, and a oldestQueuedAgeMs: null case when nothing is queued. The aggregation algorithm's own correctness (sorting, per-repo grouping, oldest-queued-age math) is already exhaustively covered by the pre-existing test/unit/miner-portfolio-dashboard.test.ts — this PR's tests verify the API handler's wiring/serialization, not re-derive that math.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no auth/session/CORS surface touched (this is a local-only, loopback dev-server API, unchanged in that respect).
  • API/OpenAPI/MCP behavior is updated and tested where needed. — the local /api/portfolio-queue dev-server endpoint's response shape changed (documented above); no public API/OpenAPI/MCP surface is touched.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — the route continues to fetch from the real local API; loading/error/empty states are unchanged, verified live against the real dev server.
  • Visible UI changes include a UI Evidence section below.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

This is a local-only, loopback dev-server dashboard with no hosted/public deployment to screenshot from this environment (no browser screenshot tooling available here). Verified functionally instead: npm run ui:test (33/33 passing, including full render assertions for the new per-repo table — column headers, one row per repo, correct per-status counts) and a live curl against the real running dev server (see Validation above) confirming the endpoint's real response shape end-to-end on a fresh install.

Notes

  • Kept the existing global summary cards (queued/in_progress/done) alongside the new per-repo table rather than replacing them — the CLI's own queue dashboard output leads with the same global totals before its per-repo table, so this mirrors that ordering.
  • oldestQueuedAgeMs is now also surfaced end-to-end (part of collectPortfolioDashboard's existing output) even though not explicitly named in the issue's acceptance criteria — it comes for free from sharing the same aggregator and further closes the CLI/UI parity gap the issue is about; not yet rendered in the UI (kept the visible scope to the requested per-repo table) but available on the typed summary for a follow-up.

…ashboard (JSONbored#4846)

The portfolio view re-implemented its own narrower, global-only
aggregation instead of reusing the CLI's own richer, per-repo
`queue dashboard` output. The local API middleware now calls
collectPortfolioDashboard directly (the same aggregator the CLI and
the read-only MCP tool already use), and the view renders a per-repo
table alongside the existing global summary cards.
@galuis116
galuis116 requested a review from JSONbored as a code owner July 12, 2026 22:35
@superagent-security

Copy link
Copy Markdown
Contributor

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

@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

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-12 22:40:11 UTC

4 files · 2 AI reviewers · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR correctly replaces the miner-ui's narrower `{ total, counts }` global aggregation with the CLI's shared `collectPortfolioDashboard` aggregator, unifying the data path across CLI, MCP tool, and UI as described. The per-repo table addition, type updates, and validation logic (`isPortfolioRepoSummary`) are consistent, and the fresh-install fast path and identifier/priority stripping are preserved correctly. Test coverage is thorough, including the wiring-only fake aggregator with a clear rationale for why the real module isn't imported directly, and CI is green.

Nits — 5 non-blocking
  • The comment block in vite-portfolio-queue-api.ts:5-17 is fairly long for a single import wiring change; could be trimmed now that the reunification story is established.
  • apps/gittensory-miner-ui/src/routes/portfolio.tsx table rows use `repo.repoFullName` as the React key, which is fine here since repo names are unique per summary, but worth a one-line note if that invariant ever changes.
  • Consider extracting the per-repo `<TableRow>` status cells into a small helper if a fourth status column is ever added, to avoid repeating `repo.byStatus.X` per column.
  • The fake `fakeCollectPortfolioDashboard` in portfolio-queue.test.tsx duplicates real aggregation logic; a short comment already explains why the real module can't be imported, which is good — no action needed.
  • nit: apps/gittensory-miner-ui/src/routes/portfolio.tsx:81 should add wrapping/truncation for `repo.repoFullName`, because long repository names can force the table wider than the card on narrow screens.
Signal Result Evidence
Code review ✅ No blockers 2 reviewers, synthesized
Linked issue ✅ Linked #4846
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: 1869 registered-repo PR(s), 1232 merged, 49 issue(s).
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1869 PR(s), 49 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — The diff closes a real duplication/regression (two divergent aggregation paths for the same store) by wiring the miner-ui to the CLI's existing shared aggregator and surfacing the richer per-repo data in the UI, which is a well-scoped, test-backed improvement tied to a linked issue.
Linked issue satisfaction

Addressed
The PR replaces the miner-ui's narrower {total, counts} aggregation with the CLI's own collectPortfolioDashboard aggregator via a shared data path, and updates the portfolio route to render a per-repo breakdown table alongside the global summary cards, matching the acceptance criterion.

Review context
  • Author: galuis116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 1869 PR(s), 49 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

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/portfolio desktop before /portfolio after /portfolio
/portfolio mobile before /portfolio (mobile) after /portfolio (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

🟩 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 added the manual-review Gittensor contributor context label Jul 12, 2026
@JSONbored
JSONbored merged commit e378706 into JSONbored:main Jul 12, 2026
8 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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reunify the miner-ui portfolio view with the CLI's own richer dashboard

2 participants