fix: publishing readiness, unit tests, and CI environment config#2
Merged
Merged
Conversation
The npm-publish workflow's debug-check job rejects console.log in src/lib. Debug output is now exclusively via the onDebugInfo callback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Found 1 test failure on Blacksmith runners: Failure
|
jaysin586
added a commit
that referenced
this pull request
Apr 28, 2026
Pre-fix, the visible-range loop translated scrollTop into messages-local coordinates with `Math.max(0, scrollTop - topGap)`, forgetting that the header sits above the messages container in the same scroll space. With a header on top, the loop walked offsets in the wrong coordinate system; when the viewport sat past every message (e.g. inside a tall footer), visibleStart fell back to 0 and the loop set visibleEnd to the last index, so the slice spanned every message and virtualization collapsed. - Extract calculateVisibleRange as a pure function in chatMeasurement, testable independently of the DOM - Subtract headerHeight as well as topGap from scrollTop - Anchor the visibleStart === -1 fallback at messages.length - 1 so overscan walks backwards from the end instead of dragging in the head - Add unit tests covering both regression paths (fix #1, fix #2) plus the standard scrolled-top / scrolled-bottom / variable-height cases
jaysin586
added a commit
that referenced
this pull request
Apr 28, 2026
Adds a realistic genai-style fixture (header inside the scroll box, tall markdown-y messages, composer rendered as a sibling outside the chat) and end-to-end coverage for the headerHeight visible-range bug. The math-level "viewport inside tall footer" case is already covered by the fix #2 unit test in chatMeasurement.svelte.test.ts. - New /tests/chat/genai-like fixture with load-50 / load-200 buttons, debug stats strip, and a composer that lives outside the scroll box - New tests/chat/genai-like.spec.ts: bounded DOM at 50 and 200 msgs, sequential anchored ids, domMessageCount parity - header-footer.spec.ts: regression test pinning the viewport at the bottom and asserting dom < 25 with 50+ messages - /tests landing page: link the new fixture under "Tests — Performance"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds all missing pieces required for npm publishing and Cloudflare docs deployment to work end-to-end. The previous PR (#1) failed CI due to missing LICENSE, missing unit tests, a
console.login src/lib, and incomplete workflow configuration.Changes
📦 Publishing readiness
console.logfrom src/lib to pass the debug-check CI jobworker-configuration.d.tsfor Cloudflare types🧪 Unit tests
ChatHeightCache,calculateTotalHeight,calculateOffsetForIndex,captureScrollAnchor,restoreScrollAnchor🔄 CI/CD fixes
cienvironment to coveralls workflowdocsenvironment to cloudflare-deploy workflowsitemap:manifestscript and generation for docs deploy📦 Dependency bumps
Commits
18737c2fix: remove console.log from src/lib to pass debug-check CIe5c872etest: add unit tests for ChatHeightCache and chatAnchoring4356d45chore: add worker-configuration.d.ts for Cloudflare types787b680build: bump svelte 5.55.4, typescript-eslint 8.58.2, wrangler 4.82.2a5d7729ci(cloudflare): expand deploy trigger paths and add pnpm cache02fa2a0build: add sitemap:manifest script for cloudflare-deploy workflowfa409daci: add docs environment to cloudflare-deploy workflowe69b539ci: add ci environment to coveralls workflow5a6dcd8chore: add MIT LICENSE file for npm publishing