Skip to content

Fix complete Code Review results presentation #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sgardoll merged 1 commit into from
Jul 26, 2026
Merged

Fix complete Code Review results presentation #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 27 additions & 11 deletions app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4665,7 +4665,7 @@ function showPaywallExhausted(count, limit, options = {}) {

const resultsView = document.getElementById('results-view')
if (resultsView) resultsView.classList.remove('visible')
document.body.classList.remove("results-fullscreen")
document.body.classList.remove("results-fullscreen", "results-with-sidebar")

const pipelineProgress = document.getElementById('pipeline-progress')
if (pipelineProgress) pipelineProgress.classList.remove('visible')
Expand Down Expand Up @@ -5601,7 +5601,7 @@ function showPipelineProgress() {

if (readyState) readyState.classList.add("hidden");
if (resultsView) resultsView.classList.remove("visible");
document.body.classList.remove("results-fullscreen");
document.body.classList.remove("results-fullscreen", "results-with-sidebar");
if (progress) progress.classList.add("visible");

pipelineStartTime = Date.now();
Expand Down Expand Up @@ -5746,18 +5746,21 @@ function renderSummaryDetail(presentation) {

const score = presentation.score;
const scoreTone = score == null
? presentation.status
? "neutral"
: score >= 80 ? "pass" : score >= 60 ? "warning" : "fail";
const scoreLabel = score == null
? "Not scored"
? ""
: score >= 80 ? "Strong" : score >= 60 ? "Needs work" : "High risk";
const findings = presentation.findings.length
? `
<ul class="summary-findings">
${presentation.findings.map((finding) => `
<li class="summary-finding-${escapeAttr(finding.severity)}">
${reviewStatusIcon(finding.severity)}
<span>${escapeHtml(finding.message)}</span>
<span>
<strong>${escapeHtml(finding.message)}</strong>
${finding.suggestion ? `<small>${escapeHtml(finding.suggestion)}</small>` : ""}
</span>
</li>
`).join("")}
</ul>
Expand Down Expand Up @@ -5791,7 +5794,7 @@ function renderSummaryDetail(presentation) {
<span class="review-score-label">Score</span>
<strong>${score == null ? "—" : escapeHtml(score)}</strong>
<span class="review-score-total">${score == null ? "Not scored" : "out of 100"}</span>
<span class="review-score-status">${escapeHtml(scoreLabel)}</span>
${scoreLabel ? `<span class="review-score-status">${escapeHtml(scoreLabel)}</span>` : ""}
</aside>
<div class="review-score-feedback">
<span>Is the generated code correct?</span>
Expand All @@ -5815,9 +5818,23 @@ function renderSelectedArtifactReview(presentation) {
) || presentation.artifacts[0];
if (!selectedArtifact) return "";

const emptyFindingState = {
pass: {
icon: "pass",
message: "No file-specific issues were found.",
},
warning: {
icon: "warning",
message: "This file needs attention, but Code Review did not return a specific finding.",
},
fail: {
icon: "fail",
message: "This file is blocked, but Code Review did not return a specific finding.",
},
}[selectedArtifact.status];
const findings = selectedArtifact.findings.length
? selectedArtifact.findings.map(renderFinding).join("")
: `<div class="review-empty-state">${reviewStatusIcon("pass")} No file-specific findings were returned.</div>`;
: `<div class="review-empty-state review-empty-${escapeAttr(selectedArtifact.status)}">${reviewStatusIcon(emptyFindingState.icon)} ${escapeHtml(emptyFindingState.message)}</div>`;
const dependencies = selectedArtifact.dependencies?.length
? selectedArtifact.dependencies.map((dependency) => `
<li><code>${escapeHtml(dependency.name)}${dependency.version ? ` ${escapeHtml(dependency.version)}` : ""}</code>${dependency.reason ? `<p>${escapeHtml(dependency.reason)}</p>` : ""}</li>
Expand Down Expand Up @@ -5936,10 +5953,7 @@ function renderBundleControls(presentation) {

function updateSelectedArtifactPanels() {
document.body.classList.add("results-fullscreen");
document.body.classList.toggle(
"results-with-sidebar",
IS_DEV && new URLSearchParams(window.location.search).get("debugSidebar") === "1",
);
document.body.classList.add("results-with-sidebar");
const resultsView = document.getElementById("results-view");
const codeOutput = document.getElementById("results-code-output");
const auditOutput = document.getElementById("results-audit-output");
Expand Down Expand Up @@ -5974,6 +5988,8 @@ function showResultsView(codeContent, auditContent) {
}
pipelineState.resultsViewMode = "summary";
document.body.classList.add("results-fullscreen");
const legacyReviewOutput = document.getElementById("step3-output");
if (legacyReviewOutput) legacyReviewOutput.textContent = "";
updateSelectedArtifactPanels();
updateDeployButtonVisibility();

Expand Down
52 changes: 40 additions & 12 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,14 @@
font-size: 12px;
font-weight: 700;
}
.review-empty-warning {
color: #92400e;
background: #fffbeb;
}
.review-empty-fail {
color: #b91c1c;
background: #fef2f2;
}
.review-muted { color: #94a3b8; font-size: 11px; }
.deploy-order-list,
.relationship-list,
Expand Down Expand Up @@ -1068,11 +1076,11 @@
.file-relationship-list li { color: #475569; font-size: 11px; }
.file-relationship-list li p { margin-top: 3px; color: #94a3b8; }
body.results-fullscreen .sidebar {
display: none;
display: flex;
}
Comment on lines 1078 to 1080

@greptile-apps greptile-apps Bot Jul 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Mobile results start below sidebar

When Results opens at 768px or narrower, this rule keeps the sidebar visible while the mobile layout stacks it above the main content. Because the sidebar has a 100dvh minimum height and Results receives another 100dvh, users must scroll past a full-screen sidebar before reaching the review.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: index.html
Line: 1078-1080

Comment:
**Mobile results start below sidebar**

When Results opens at 768px or narrower, this rule keeps the sidebar visible while the mobile layout stacks it above the main content. Because the sidebar has a 100dvh minimum height and Results receives another 100dvh, users must scroll past a full-screen sidebar before reaching the review.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

body.results-fullscreen .main-content {
display: block;
width: 100vw;
width: auto;
height: 100dvh;
padding: 0;
}
Expand All @@ -1086,13 +1094,13 @@
}
body.results-fullscreen.results-with-sidebar .main-content {
width: auto;
height: 100vh;
padding: 36px;
height: 100dvh;
padding: 0;
}
body.results-fullscreen.results-with-sidebar .main-stage {
inset: 28px;
border-radius: 14px;
box-shadow: 0 1px 3px rgba(0,0,0,0.06), 0 6px 24px rgba(0,0,0,0.05);
inset: 0;
border-radius: 0;
box-shadow: none;
}
body.results-fullscreen .results-header {
padding: 14px 28px;
Expand Down Expand Up @@ -1149,19 +1157,25 @@
letter-spacing: -0.025em;
}
body.results-fullscreen .results-summary-detail {
flex: 0 1 auto;
max-height: 340px;
padding: 26px 36px 24px;
flex: 1 1 auto;
max-height: none;
padding: 28px 36px;
overflow-y: auto;
border-bottom: 1px solid #e5e7eb;
background: #f8fafc;
}
.review-summary {
display: grid;
grid-template-columns: minmax(0, 1fr) 184px;
gap: 48px;
gap: 36px;
width: 100%;
max-width: 1280px;
max-width: none;
margin: 0 auto;
padding: 24px;
border: 1px solid #e2e8f0;
border-radius: 12px;
background: #fff;
box-shadow: 0 8px 24px -20px rgba(15, 23, 42, 0.28);
}
.review-summary-copy { min-width: 0; }
.results-file-count {
Expand Down Expand Up @@ -1200,6 +1214,7 @@
.review-score-pass { color: #166534; border-color: #86efac; background: #f0fdf4; }
.review-score-warning { color: #9a3412; border-color: #fdba74; background: #fff7ed; }
.review-score-fail { color: #991b1b; border-color: #fca5a5; background: #fef2f2; }
.review-score-neutral { color: #475569; border-color: #cbd5e1; background: #f8fafc; }
.review-score-label {
font-size: 11px;
font-weight: 800;
Expand Down Expand Up @@ -1274,6 +1289,19 @@
font-size: 14px;
line-height: 1.45;
}
.summary-findings li > span {
display: grid;
gap: 3px;
}
.summary-findings li strong {
color: #334155;
font-weight: 700;
}
.summary-findings li small {
color: #64748b;
font-size: 12px;
line-height: 1.45;
}
.summary-findings .review-status-icon { margin-top: 2px; }
.review-score-feedback {
width: 100%;
Expand Down
5 changes: 5 additions & 0 deletions src/pipelineContracts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ export function buildReviewPrompt(generatedBundle) {
return stringifyPipelinePayload({
task: "review_bundle",
generatedBundle: parseJsonIfPossible(generatedBundle),
outputRequirements: {
overall: ["status", "score", "summary", "findings"],
scoreRange: [0, 100],
eachArtifact: ["id", "review.status", "review.findings"],
},
});
}

Expand Down
12 changes: 12 additions & 0 deletions src/pipelineContracts.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ test("review prompt sends generated bundle data without system instructions", ()

assert.equal(payload.task, "review_bundle");
assert.equal(payload.generatedBundle.artifacts[0].id, "action-a");
assert.deepEqual(payload.outputRequirements.overall, [
"status",
"score",
"summary",
"findings",
]);
assert.deepEqual(payload.outputRequirements.scoreRange, [0, 100]);
assert.deepEqual(payload.outputRequirements.eachArtifact, [
"id",
"review.status",
"review.findings",
]);
assert.doesNotMatch(prompt, /Review every generated artifact/);
assert.doesNotMatch(prompt, /bundleReview/);
});
Expand Down
12 changes: 4 additions & 8 deletions src/reviewPresentation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function normalizeReviewStatus(value) {
const status = String(value || "").toLowerCase();
if (/(fail|error|critical|block|reject|invalid)/.test(status)) return "fail";
if (/(warn|attention|manual|partial|incomplete|concern)/.test(status)) return "warning";
if (/(pass|success|ready|approve|valid|clean)/.test(status)) return "pass";
if (/(pass|success|ready|approve|valid|clean|info|notice)/.test(status)) return "pass";
return null;
}

Expand Down Expand Up @@ -177,12 +177,7 @@ function unwrapReviewPayload(reviewResult) {
}

const root = toObject(value);
const nested = toObject(
root.bundleReview
|| root.reviewResult
|| root.codeReview
|| root.result,
);
const nested = toObject(root.reviewResult || root.codeReview || root.result);
return { rawText: "", root: Object.keys(nested).length ? nested : root };
}

Expand Down Expand Up @@ -268,7 +263,8 @@ export function buildReviewPresentation({ bundle, reviewResult }) {
const artifacts = asArray(safeBundle.artifacts);
const { root, rawText } = unwrapReviewPayload(reviewResult);
const overallReview = toObject(
root.overallReview
root.bundleReview
|| root.overallReview
|| root.overall
|| root.bundleSummary
|| root.summaryReview
Expand Down
56 changes: 56 additions & 0 deletions src/reviewPresentation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,59 @@ test("leaves structured reviews without score fields unscored", () => {

assert.equal(presentation.score, null);
});

test("preserves top-level artifact reviews alongside bundleReview", () => {
const presentation = buildReviewPresentation({
bundle,
reviewResult: {
artifacts: [
{
id: "format-payload",
review: {
status: "pass",
findings: [
{
severity: "info",
message: "Formatting logic is valid.",
suggestion: "No changes required.",
},
],
},
},
{
id: "execute-webhook",
review: {
status: "pass",
findings: [
{
severity: "info",
message: "Webhook execution is valid.",
suggestion: "No changes required.",
},
],
},
},
],
bundleReview: {
status: "pass",
summary: "The complete bundle is ready to deploy.",
score: 96,
findings: [
{
severity: "info",
message: "The deploy order is correct.",
suggestion: "Deploy the files in the supplied order.",
},
],
},
},
});

assert.equal(presentation.status, "pass");
assert.equal(presentation.score, 96);
assert.equal(presentation.summary, "The complete bundle is ready to deploy.");
assert.equal(presentation.findings[0].message, "The deploy order is correct.");
assert.deepEqual(presentation.counts, { pass: 2, warning: 0, fail: 0 });
assert.equal(presentation.reviewCoverage.reviewed, 2);
assert.equal(presentation.artifacts[0].findings[0].severity, "pass");
});