Enhance Fake review#69
Merged
Merged
Conversation
app.js is embedded into the binary and used for all real reviews, not
just fake mode.
The change is safe for real reviews, and actually fixes a latent bug
there too. Here's why it didn't visibly break real reviews before:
Real review flow:
1. Comments are streamed progressively via events →
appendStreamedCommentsToFiles merges them into prev.files
2. When completed, fetchFinalReviewData fires → tries
/api/v1/diff-review/{reviewID} on the LiveReview server first
3. That returns the final review with all comments in data.files
4. With the old code: prev.files was used — it already had the streamed
comments, so the result looked correct
5. With the new code: data.files is used — same comments, same result
Why it only broke in fake mode:
- Fake events (buildFakeEventsResponse) never stream any comments — only
log/batch/status/completion events
- So prev.files never got any comments merged in
- The final /api/review fetch had the comments in data.files, but the
old code discarded them in favor of the comment-less prev.files
The new code is strictly more correct: the final fetch is explicitly
there to get the definitive completed state. That response's data.files
should always be preferred. It falls back to prev.files only if
data.files is null/undefined (e.g. the server returned no files field at
all), which is the correct fallback.
LiveReview Pre-Commit Check: ran (iter:12, coverage:94%)
LiveReview Pre-Commit Check: ran (iter:1, coverage:0%)
Add LRC_STATIC_DEV_DIR env var support to the staticserve package. When set, all three serving paths (GetStaticHandler, ReadFile, RenderPreactHTML) read from the filesystem instead of the embedded FS. Add dev-ui Makefile target that sets LRC_STATIC_DEV_DIR to the static source directory and runs the fake review with a 5s wait. Editing any JS file in internal/staticserve/static/ is visible on browser refresh without rebuilding the binary.
Wrap the dev mode file server with Cache-Control: no-store so the browser always fetches JS files fresh from disk on every refresh, without needing a hard refresh (Ctrl+Shift+R).
LinceMathew
reviewed
May 18, 2026
| type JSONCommentData = result.JSONCommentData | ||
|
|
||
| func devStaticDir() string { | ||
| return os.Getenv("LRC_STATIC_DEV_DIR") |
Contributor
There was a problem hiding this comment.
what is this new env? where we need to configure this?
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
2 participants
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.
Three improvements to the fake review E2E testing mode (make run-fake-review):
What changed
internal/appcore/bridge.go
Replaced buildSyntheticCommentsByFile with a per-file spec table (perFileCommentSpecs). Each fake file gets its own comment(s) with line numbers resolved from actual diff hunks at runtime:
┌───────────────────────────────┬────────────┐
│ File │ Comments │
├───────────────────────────────┼────────────┤
│ README.md │ 1 Critical │
├───────────────────────────────┼────────────┤
│ src/edge_cases.txt │ 2 Error │
├───────────────────────────────┼────────────┤
│ src/fake_large_config.toml │ 1 Warning │
├───────────────────────────────┼────────────┤
│ src/only_one_line.txt │ 1 Info │
├───────────────────────────────┼────────────┤
│ src/ui_connectors_handlers.go │ 1 Info │
└───────────────────────────────┴────────────┘
internal/staticserve/static/app.js
Fixed a latent bug in fetchFinalReviewData: the merge used prev.files || data.files, which kept the stale comment-less initial files because they were truthy. Changed to data.files || prev.files so the definitive final-fetch response takes precedence. This fix applies to all reviews (real and fake) — it was invisible in real mode because real reviews stream comments via events before the final fetch, so prev.files already had them.
review/fake_mode.go
Updated BuildFakeCompletedResult summary to include the three sections (Overview, Technical Highlights, Impact) that evaluateSummarySlidesEligibility requires. The slideshow now activates with ~7 hardcoded slides — no LLM call involved.