Skip to content

test(miner-ui): cover chat-scroll stick-to-bottom boundary and clamp (#7793) - #7848

Closed
galuis116 wants to merge 1 commit into
JSONbored:mainfrom
galuis116:test/chat-scroll-boundary-coverage
Closed

test(miner-ui): cover chat-scroll stick-to-bottom boundary and clamp (#7793)#7848
galuis116 wants to merge 1 commit into
JSONbored:mainfrom
galuis116:test/chat-scroll-boundary-coverage

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Summary

Closes #7793.

apps/loopover-miner-ui/src/lib/chat-scroll.ts encodes the <= 80px near-bottom threshold (CHAT_NEAR_BOTTOM_PX) and the clamp used by the chat stick-to-bottom auto-scroll, but had no direct test. Its only consumer, components/chat/message-list.tsx, is covered for DOM structure / StateBoundary branches only - and jsdom does not meaningfully simulate scrollTop / scrollHeight there, so the scroll math was never actually exercised. Per git log this logic already needed one dedicated bug-fix pass (#7229/#7298), which is exactly the class of regression a unit test catches.

Adds chat-scroll.test.ts next to the module, following the one-test-per-pure-module convention demo-data.test.ts already establishes in the same directory:

isChatViewportNearBottom

  • distance of exactly CHAT_NEAR_BOTTOM_PX -> true (the <= boundary that motivated the prior fix)
  • one pixel past the threshold -> false
  • sitting exactly at the bottom (distance 0) -> true
  • content shorter than the viewport (negative distance, nothing to scroll) -> true
  • scrolled far up -> false
  • an explicit thresholdPx overriding the default (both arms, including 0 where <= still admits distance 0)
  • CHAT_NEAR_BOTTOM_PX is the documented 80

scrollChatViewportToBottom

  • scrolls to the maximum scrollable offset
  • clamps to 0 when the content is shorter than the viewport (the Math.max(0, ...) arm)
  • is idempotent once already bottomed

Test-only, no production change.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • 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 a currently open issue this PR resolves (Closes #7793).

Validation

  • git diff --check
  • npm run actionlint - no workflow files touched.
  • npm run ui:typecheck (clean across both UI workspaces)
  • Full @loopover/ui-miner suite: 33 files / 378 tests pass, including the 10 new ones.
  • npm run test:coverage / test:workers / build:mcp / test:mcp-pack / ui:openapi:check - not run: this adds a single test file under apps/loopover-miner-ui/ and touches no backend, worker, MCP, or OpenAPI surface.
  • npm audit --audit-level=moderate - no dependency changes.
  • New behavior has unit tests for new branches and fallback paths - this PR is that test coverage.

If any required check was skipped, explain why:

  • Test-only change with no src/** lines modified, so codecov/patch has no changed lines to score. Per the issue, apps/loopover-miner-ui is outside the src/** 99% patch gate - this new test file is the issue's own coverage deliverable.
  • ui:lint could not be run meaningfully on this Windows checkout: core.autocrlf=true makes prettier report Delete CR on every file in the repo (including files this PR never touches), so its output is not a signal here. The new file is written with LF endings and is clean under eslint once that CRLF noise is excluded.

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 and low-noise.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests - n/a, none touched.
  • API/OpenAPI/MCP behavior is updated and tested where needed - n/a.
  • UI changes use live API data or real empty/error/loading states - n/a, no component or rendering change.
  • Visible UI changes include a UI Evidence section - n/a: this is a test-only change with no visible UI difference.
  • Public docs/changelogs are updated where needed - n/a.

Notes

  • The fake viewport is a plain object cast to HTMLElement rather than a real DOM node, because jsdom cannot set scrollHeight / clientHeight on a real element - which is precisely why the existing message-list test could not cover this math.

…7793)

apps/loopover-miner-ui/src/lib/chat-scroll.ts encodes the 80px near-bottom
threshold and the scroll clamp for the chat auto-scroll feature, but had no
direct test: its only consumer (components/chat/message-list.tsx) is covered
for DOM structure, and jsdom does not meaningfully simulate scrollTop /
scrollHeight there. This logic already needed one dedicated bug-fix pass
(#7229/#7298), which is exactly the kind of regression a unit test catches.

Adds chat-scroll.test.ts following the one-test-per-pure-module convention
demo-data.test.ts already establishes in the same directory:

- isChatViewportNearBottom: the <= boundary at exactly CHAT_NEAR_BOTTOM_PX,
  one pixel past it, sitting exactly at the bottom, content shorter than the
  viewport (negative distance), scrolled far up, and an explicit thresholdPx
  overriding the default.
- scrollChatViewportToBottom: scrolls to the maximum offset, clamps to 0 when
  the content is shorter than the viewport, and is idempotent once bottomed.

Test-only, no production change.
@galuis116
galuis116 requested a review from JSONbored as a code owner July 21, 2026 15:00
@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 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-21 15:07:22 UTC

1 file · 1 AI reviewer · 1 blocker · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
Adds a focused unit test file for the pure `chat-scroll.ts` module, directly testing `isChatViewportNearBottom` and `scrollChatViewportToBottom` against the real functions shown in the FULL FILE CONTENT, including the documented `<= 80px` boundary, negative-distance, custom-threshold, and clamp-to-zero arms. The test logic matches the actual implementation exactly (e.g. distance = scrollHeight - scrollTop - clientHeight, verified against the real formula), it's test-only with no production change, closes the linked issue #7793, and CI passed.

Nits — 5 non-blocking
  • The `viewport()` helper casts a plain object to `HTMLElement` via `as unknown as HTMLElement`, which is fine here but slightly less realistic than the `Object.defineProperty` getter/setter approach used in the sibling `chat-message-components.test.tsx` for scrollTop mutation — worth a one-line comment noting `scrollChatViewportToBottom` never reads back a live setter in this file.
  • Consider adding a case where `thresholdPx` is negative or where scrollHeight equals clientHeight exactly (distance 0) to further pin the boundary semantics, though this is optional given the existing 'sits exactly at the bottom' case already covers distance 0.
  • None needed beyond the nits above — the test set is thorough and well-targeted for a pure module with a documented boundary bug history (Chat rail never auto-scrolls to a new message or a live streaming response #7229/fix(miner-ui): chat rail stick-to-bottom auto-scroll #7298).
  • Pull request duplicates other open work — Check for an existing pull request or issue covering this change and coordinate or consolidate before continuing.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.

Why this is blocked

📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Linked issue overlaps another open PR: Other open pull requests reference the same linked issue set: #7847. — Review the related PRs before spending reviewer time on duplicate work.

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7793
Related work ⚠️ Same linked issue: #7847 Another open PR references the same linked issue.
Change scope ❌ 8/20 High 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: 1946 registered-repo PR(s), 1275 merged, 53 issue(s).
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1946 PR(s), 53 issue(s).
Improvement ℹ️ Insufficient signal risk: low · value: insufficient-signal · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds chat-scroll.test.ts directly testing isChatViewportNearBottom's <=80px boundary (both sides), zero-distance and negative-distance cases, an explicit thresholdPx override, and scrollChatViewportToBottom's max-offset/clamp/idempotent behavior, matching the issue's deliverable and convention.

Review context
Contributor next steps
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 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (Linked issue overlaps another open PR; duplicate of open PR #7847). 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 21, 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.

chat-scroll.ts (auto-scroll stick-to-bottom logic) has zero test coverage despite a prior dedicated bug-fix PR

1 participant