Merge pull request #69 from HexmosTech/fake-review

Enhance Fake review

Author: lovestaco

Makefile

@@ -1,4 +1,4 @@
-.PHONY: build build-win build-all build-local build-local-test run run-fake-review bump release release-internal release-gh clean test testall test-pkg upload-secrets download-secrets security-govulncheck security-govulncheck-json security-osv security-triage security-gitleaks security-b2-audit security-b2-cleanup-plan security-b2-cleanup-apply security-publish-release-manifest security-secret-regression security-sbom security-sbom-cyclonedx security-sbom-spdx security-sbom-validate release-notes-init release-notes-check release-preflight
+.PHONY: build build-win build-all build-local build-local-test run run-fake-review dev-ui bump release release-internal release-gh clean test testall test-pkg upload-secrets download-secrets security-govulncheck security-govulncheck-json security-osv security-triage security-gitleaks security-b2-audit security-b2-cleanup-plan security-b2-cleanup-apply security-publish-release-manifest security-secret-regression security-sbom security-sbom-cyclonedx security-sbom-spdx security-sbom-validate release-notes-init release-notes-check release-preflight
 
 # Go parameters
 GOENV=env -u GOROOT
@@ -70,6 +70,14 @@ run: build-local
 run-fake-review: build-local-test
 	@WAIT=$${WAIT:-30s} TMP_REPO=$${TMP_REPO:-/tmp/lrc-fake-review-repo} scripts/fake_review.sh $(ARGS)
 
+# Run fake review with live JS reloading — edit files in internal/staticserve/static/, refresh browser
+# No rebuild needed after JS changes: just edit and refresh the browser tab.
+dev-ui: build-local-test
+	@LRC_STATIC_DEV_DIR=$(CURDIR)/internal/staticserve/static \
+	 WAIT=$${WAIT:-5s} \
+	 TMP_REPO=$${TMP_REPO:-/tmp/lrc-fake-review-repo} \
+	 scripts/fake_review.sh $(ARGS)
+
 # Bump lrc version by editing appVersion in main.go
 # Prompts for version bump type (patch/minor/major)
 bump:

internal/appcore/bridge.go

@@ -113,7 +113,7 @@ func buildFakeCompletedResultForFiles(baseFiles []reviewmodel.DiffReviewFileResu
 
 	if totalComments > 0 {
 		result.Summary = fmt.Sprintf(
-			"%s\n\n## Synthetic Coverage\n\n- Generated %d synthetic comment(s) across %d file(s)\n- Includes deterministic Copy Issue scenarios: first-line (no prev), last-line (no next), interior-line (both prev/next)",
+			"%s\n\n## Synthetic Coverage\n\n- Generated %d synthetic comment(s) across %d file(s)\n- Covers Critical, Error, Warning, and Info severities across targeted files",
 			strings.TrimSpace(result.Summary),
 			totalComments,
 			len(files),
@@ -124,88 +124,106 @@ func buildFakeCompletedResultForFiles(baseFiles []reviewmodel.DiffReviewFileResu
 	return result
 }
 
+type syntheticCommentSpec struct {
+	linePickIndex int // -1 = last available line, ≥0 = Nth available line
+	severity      string
+	category      string
+	content       string
+}
+
+// perFileCommentSpecs maps each fake review file's base name to the comments to
+// generate for it. Line numbers are resolved at runtime from actual diff hunks.
+var perFileCommentSpecs = map[string][]syntheticCommentSpec{
+	"README.md": {
+		{
+			linePickIndex: 0,
+			severity:      "Critical",
+			category:      "Documentation",
+			content:       "README is missing required Go version and platform prerequisites — document these before shipping.",
+		},
+	},
+	"edge_cases.txt": {
+		{
+			linePickIndex: 0,
+			severity:      "Error",
+			category:      "Logic",
+			content:       "`alpha-updated` is inconsistent with downstream parser expectations; update the canonical test fixture.",
+		},
+		{
+			linePickIndex: -1,
+			severity:      "Error",
+			category:      "Logic",
+			content:       "`delta-updated` does not match the expected integration test output — realign the test data.",
+		},
+	},
+	"fake_large_config.toml": {
+		{
+			linePickIndex: 0,
+			severity:      "Warning",
+			category:      "Configuration",
+			content:       "`enable_telemetry = true` in a generated config risks leaking test data to analytics endpoints — disable for local runs.",
+		},
+	},
+	"only_one_line.txt": {
+		{
+			linePickIndex: 0,
+			severity:      "Info",
+			category:      "Style",
+			content:       "Single-line file — confirm the seed suffix is stable enough for snapshot testing.",
+		},
+	},
+	"ui_connectors_handlers.go": {
+		{
+			linePickIndex: 0,
+			severity:      "Info",
+			category:      "Style",
+			content:       "`normalizeConnectorName` chains three sequential string operations; consider combining into a single `strings.Map` pass for clarity.",
+		},
+	},
+}
+
 func buildSyntheticCommentsByFile(files []reviewmodel.DiffReviewFileResult) map[string][]reviewmodel.DiffReviewComment {
 	commentsByFile := make(map[string][]reviewmodel.DiffReviewComment)
 
-	// Primary scenario: find one hunk that has enough lines to guarantee
-	// first-line, last-line, and interior-line comments.
 	for _, file := range files {
-		for _, hunk := range file.Hunks {
-			numbers := collectHunkAddedLineNumbers(hunk)
-			if len(numbers) < 3 {
-				continue
-			}
-
-			interiorIdx := 1
-			if interiorIdx >= len(numbers)-1 {
-				interiorIdx = len(numbers) / 2
-			}
-			if interiorIdx <= 0 {
-				interiorIdx = 1
-			}
-			if interiorIdx >= len(numbers)-1 {
-				interiorIdx = len(numbers) - 2
-			}
-
-			commentsByFile[file.FilePath] = []reviewmodel.DiffReviewComment{
-				{
-					Line:     numbers[0],
-					Severity: "Warning",
-					Category: "Context",
-					Content:  "Hunk-start line: verify Copy Issue handles missing previous line context correctly.",
-				},
-				{
-					Line:     numbers[len(numbers)-1],
-					Severity: "Error",
-					Category: "Context",
-					Content:  "Hunk-end line: verify Copy Issue handles missing next line context correctly.",
-				},
-				{
-					Line:     numbers[interiorIdx],
-					Severity: "Info",
-					Category: "Context",
-					Content:  "Interior line: verify Copy Issue includes both previous and next lines in the code excerpt.",
-				},
-			}
-			return commentsByFile
+		base := file.FilePath
+		if idx := strings.LastIndex(base, "/"); idx >= 0 {
+			base = base[idx+1:]
+		}
+		specs, ok := perFileCommentSpecs[base]
+		if !ok {
+			continue
 		}
-	}
 
-	// Fallback for very small diffs: emit whatever subset is possible.
-	for _, file := range files {
+		var allLines []int
 		for _, hunk := range file.Hunks {
-			numbers := collectHunkAddedLineNumbers(hunk)
-			if len(numbers) == 0 {
-				continue
-			}
+			allLines = append(allLines, collectHunkAddedLineNumbers(hunk)...)
+		}
+		if len(allLines) == 0 {
+			continue
+		}
 
-			comments := []reviewmodel.DiffReviewComment{
-				{
-					Line:     numbers[0],
-					Severity: "Warning",
-					Category: "Context",
-					Content:  "Hunk-start line: verify Copy Issue handles missing previous line context correctly.",
-				},
+		var comments []reviewmodel.DiffReviewComment
+		for _, spec := range specs {
+			idx := spec.linePickIndex
+			if idx < 0 {
+				idx = len(allLines) + idx
 			}
-			if len(numbers) > 1 {
-				comments = append(comments, reviewmodel.DiffReviewComment{
-					Line:     numbers[len(numbers)-1],
-					Severity: "Error",
-					Category: "Context",
-					Content:  "Hunk-end line: verify Copy Issue handles missing next line context correctly.",
-				})
+			if idx < 0 {
+				idx = 0
 			}
-			if len(numbers) > 2 {
-				comments = append(comments, reviewmodel.DiffReviewComment{
-					Line:     numbers[1],
-					Severity: "Info",
-					Category: "Context",
-					Content:  "Interior line: verify Copy Issue includes both previous and next lines in the code excerpt.",
-				})
+			if idx >= len(allLines) {
+				idx = len(allLines) - 1
 			}
-
+			comments = append(comments, reviewmodel.DiffReviewComment{
+				Line:     allLines[idx],
+				Severity: spec.severity,
+				Category: spec.category,
+				Content:  spec.content,
+			})
+		}
+		if len(comments) > 0 {
 			commentsByFile[file.FilePath] = comments
-			return commentsByFile
 		}
 	}
 

internal/staticserve/static/app.js

@@ -332,7 +332,7 @@ async function initApp() {
                         return {
                             ...prev,
                             ...data,
-                            files: prev.files || data.files || []
+                            files: data.files || prev.files || []
                         };
                     });
                     return data;

internal/staticserve/static_serve.go

@@ -2,7 +2,12 @@ package staticserve
 
 import (
 	"embed"
+	"encoding/json"
+	"mime"
 	"net/http"
+	"os"
+	"path/filepath"
+	"strings"
 
 	"github.com/HexmosTech/git-lrc/result"
 )
@@ -16,13 +21,42 @@ type JSONHunkData = result.JSONHunkData
 type JSONLineData = result.JSONLineData
 type JSONCommentData = result.JSONCommentData
 
+func devStaticDir() string {
+	return os.Getenv("LRC_STATIC_DEV_DIR")
+}
+
 // RenderPreactHTML renders the Preact-based HTML with embedded JSON data.
 func RenderPreactHTML(data *result.HTMLTemplateData) (string, error) {
+	if dir := devStaticDir(); dir != "" {
+		jsonData := result.ConvertToJSONData(data)
+		jsonBytes, err := json.Marshal(jsonData)
+		if err != nil {
+			return "", err
+		}
+		htmlBytes, err := os.ReadFile(filepath.Join(dir, "index.html"))
+		if err != nil {
+			return "", err
+		}
+		html := strings.Replace(string(htmlBytes), "{{.JSONData}}", string(jsonBytes), 1)
+		if data.FriendlyName != "" {
+			html = strings.Replace(html, "<title>LiveReview Results</title>",
+				"<title>LiveReview Results — "+data.FriendlyName+"</title>", 1)
+		}
+		return html, nil
+	}
 	return result.RenderPreactHTML(data, staticFiles)
 }
 
 // GetStaticHandler returns an HTTP handler for serving static files.
 func GetStaticHandler() http.Handler {
+	if dir := devStaticDir(); dir != "" {
+		_ = mime.AddExtensionType(".mjs", "application/javascript; charset=utf-8")
+		fs := http.FileServer(http.Dir(dir))
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Cache-Control", "no-store")
+			fs.ServeHTTP(w, r)
+		})
+	}
 	return result.GetStaticHandler(staticFiles)
 }
 
@@ -31,7 +65,10 @@ func ServeStaticFile(w http.ResponseWriter, r *http.Request, filename string) er
 	return result.ServeStaticFile(w, filename, staticFiles)
 }
 
-// ReadFile reads a file from the embedded static directory.
+// ReadFile reads a file from the embedded static directory (or filesystem in dev mode).
 func ReadFile(name string) ([]byte, error) {
+	if dir := devStaticDir(); dir != "" {
+		return os.ReadFile(filepath.Join(dir, name))
+	}
 	return staticFiles.ReadFile("static/" + name)
 }

review/fake_mode.go

@@ -48,7 +48,25 @@ func BuildFakeSubmitResponse(now time.Time, friendlyName string) CreateResponse
 
 func BuildFakeCompletedResult() *Response {
 	return &Response{
-		Status:  "completed",
-		Summary: "# Fake Review Summary\n\nThis is a fake review generated by `build-local-test` for end-to-end testing.\n\n- No AI request was sent\n- Review data is synthetic and deterministic",
+		Status: "completed",
+		Summary: `# Fake Review Summary
+
+This is a fake review generated by ` + "`build-local-test`" + ` for end-to-end testing.
+
+- No AI request was sent
+- Review data is synthetic and deterministic
+
+## Overview
+
+The connector handler refactor adds empty-payload validation and provider-name normalization. Configuration scaffolding and edge-case fixtures have been updated to extend test coverage.
+
+## Technical Highlights
+
+- ` + "`src/ui_connectors_handlers.go`" + `: ` + "`handleConnector`" + ` now rejects empty payloads and injects a default ` + "`provider`" + ` key, closing a class of silent dispatch failures.
+- ` + "`src/fake_large_config.toml`" + `: New config scaffold added; confirm ` + "`enable_telemetry = true`" + ` is intentional before merging to a shared environment.
+
+## Impact
+
+Risk: Enabling telemetry in a generated config file may expose test-run metadata to analytics endpoints — scope this flag to local-only builds before promoting to CI.`,
 	}
 }