perf(preview): symlink static deps#545
Open
iamgio wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughAdds a symlink-first option for materializing script and third-party files in preview: new pipeline flag and artifact property, IO utilities for safe symlinking, exporter logic that prefers symlinks with checksum-copy fallback, rendering wiring, tests, and changelog updates. ChangesSymlink-based Dependency Linking
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
iamgio
force-pushed
the
06-08-perf_preview_symlink_static_deps
branch
3 times, most recently
from
f611364 to
37d7f78
Compare
iamgio
force-pushed
the
06-08-perf_preview_symlink_static_deps
branch
from
37d7f78 to
827762f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 19-23: Rewrite the "#### Faster live preview" changelog entry to
be fully user-facing: describe benefits (faster preview updates, reduced disk
usage, and more reliable preview transport) and add a documentation link to
https://quarkdown.com/wiki/Page; keep implementation details minimal (if needed,
put them parenthetically) and replace phrases like "symbolic links" and
"checksum-optimized" with user-centered wording such as "references libraries
via links (reducing duplicate files)"; also mention the transport change as "now
uses Server-Sent Events (SSE) for more reliable browser previews" and include
the wiki link inline for users to read more.
In `@quarkdown-test/src/test/kotlin/com/quarkdown/test/HtmlOutputResourceTest.kt`:
- Around line 337-361: The tests `preview mode marks third-party libraries as
symlinks` and `non-preview mode copies third-party libraries instead of
symlinking` rely on a default empty `source = ""`; update both tests to provide
an explicit fixture source that requires a known third-party library so
`getThirdPartyLibraryArtifacts(group)` is guaranteed non-empty, keep using
`execute(..., previewMode = true|false, outputResourceHook = { group -> ... })`;
also add analogous assertions for the script resource path by calling the script
artifact accessor (e.g., getScriptArtifacts(group) or the project’s script
artifact helper) and assert its `symlink` flag matches the expected behavior
(true in previewMode test, false in non-preview test) so both third-party and
script wiring are covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d04e0b7-30b8-418d-a4a8-b1207d13ed6e
📒 Files selected for processing (12)
CHANGELOG.mdquarkdown-core/src/main/kotlin/com/quarkdown/core/pipeline/PipelineOptions.ktquarkdown-core/src/main/kotlin/com/quarkdown/core/pipeline/output/FileReferenceOutputArtifact.ktquarkdown-core/src/main/kotlin/com/quarkdown/core/pipeline/output/visitor/FileResourceExporter.ktquarkdown-core/src/main/kotlin/com/quarkdown/core/util/IOUtils.ktquarkdown-core/src/test/kotlin/com/quarkdown/core/FileResourceExporterTest.ktquarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/HtmlPostRenderer.ktquarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/resources/ScriptPostRendererResource.ktquarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/resources/ThirdPartyPostRendererResource.ktquarkdown-html/src/test/e2e/live-preview/live-preview.spec.tsquarkdown-install-layout-navigator/src/main/kotlin/com/quarkdown/installlayout/InstallLayoutEntry.ktquarkdown-test/src/test/kotlin/com/quarkdown/test/HtmlOutputResourceTest.kt
iamgio
force-pushed
the
06-08-perf_preview_symlink_static_deps
branch
from
827762f to
bd7349b
Compare
iamgio
force-pushed
the
06-08-perf_preview_symlink_static_deps
branch
from
bd7349b to
6c20216
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
quarkdown-test/src/test/kotlin/com/quarkdown/test/HtmlOutputResourceTest.kt (1)
349-375:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake symlink tests deterministic by using an explicit third-party fixture source.
These assertions still depend on
source = ""implicitly producinglibartifacts. Please switch both tests to a source that explicitly requires a known third-party library so failures only reflect symlink wiring, not default-library behavior drift. Line 356/Line 370 currently guard emptiness, but they don’t remove this coupling.As per coding guidelines, “Aim for a test-driven development (TDD) approach when possible; add or update tests when making changes to the compiler or other modules; ensure high test coverage across unit tests, integration tests, and E2E tests.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@quarkdown-test/src/test/kotlin/com/quarkdown/test/HtmlOutputResourceTest.kt` around lines 349 - 375, Replace the implicit empty-source usage in both tests with an explicit third-party fixture source so the tests deterministically produce third-party artifacts: update the two execute(...) calls in the tests `preview mode marks third-party libraries and script as symlinks` and `non-preview mode copies third-party libraries and script instead of symlinking` to pass a source that imports a known third-party library (so getThirdPartyLibraryArtifacts(group) will always return the expected libs), leaving the existing assertions and hooks (execute, getThirdPartyLibraryArtifacts, getScriptArtifact) unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@quarkdown-test/src/test/kotlin/com/quarkdown/test/HtmlOutputResourceTest.kt`:
- Around line 349-375: Replace the implicit empty-source usage in both tests
with an explicit third-party fixture source so the tests deterministically
produce third-party artifacts: update the two execute(...) calls in the tests
`preview mode marks third-party libraries and script as symlinks` and
`non-preview mode copies third-party libraries and script instead of symlinking`
to pass a source that imports a known third-party library (so
getThirdPartyLibraryArtifacts(group) will always return the expected libs),
leaving the existing assertions and hooks (execute,
getThirdPartyLibraryArtifacts, getScriptArtifact) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d5624482-f954-478d-9840-89dde0fe9b9e
📒 Files selected for processing (13)
CHANGELOG.mdquarkdown-core/src/main/kotlin/com/quarkdown/core/pipeline/PipelineOptions.ktquarkdown-core/src/main/kotlin/com/quarkdown/core/pipeline/output/FileReferenceOutputArtifact.ktquarkdown-core/src/main/kotlin/com/quarkdown/core/pipeline/output/visitor/FileResourceExporter.ktquarkdown-core/src/main/kotlin/com/quarkdown/core/util/IOUtils.ktquarkdown-core/src/test/kotlin/com/quarkdown/core/FileResourceExporterTest.ktquarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/HtmlPostRenderer.ktquarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/resources/ScriptPostRendererResource.ktquarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/resources/ThirdPartyPostRendererResource.ktquarkdown-html/src/test/e2e/live-preview/live-preview.spec.tsquarkdown-html/src/test/kotlin/com/quarkdown/rendering/html/HtmlResourceGenerationTest.ktquarkdown-install-layout-navigator/src/main/kotlin/com/quarkdown/installlayout/InstallLayoutEntry.ktquarkdown-test/src/test/kotlin/com/quarkdown/test/HtmlOutputResourceTest.kt
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (10)
- quarkdown-html/src/test/kotlin/com/quarkdown/rendering/html/HtmlResourceGenerationTest.kt
- quarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/resources/ScriptPostRendererResource.kt
- quarkdown-html/src/test/e2e/live-preview/live-preview.spec.ts
- quarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/resources/ThirdPartyPostRendererResource.kt
- quarkdown-core/src/main/kotlin/com/quarkdown/core/pipeline/output/FileReferenceOutputArtifact.kt
- quarkdown-core/src/main/kotlin/com/quarkdown/core/pipeline/output/visitor/FileResourceExporter.kt
- quarkdown-install-layout-navigator/src/main/kotlin/com/quarkdown/installlayout/InstallLayoutEntry.kt
- quarkdown-core/src/main/kotlin/com/quarkdown/core/util/IOUtils.kt
- quarkdown-html/src/main/kotlin/com/quarkdown/rendering/html/post/HtmlPostRenderer.kt
- quarkdown-core/src/test/kotlin/com/quarkdown/core/FileResourceExporterTest.kt
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.
Closes #ISSUE_NUMBERat the end of this PR description.docsandCHANGELOG.mdSummary by CodeRabbit
Documentation
New Features
Tests