Back to feed

sledtools/pika branch #30

pika-git-ui-1

Polish branch review layout and tutorial warnings

Target branch: master

Merge Commit: 38fdcc8ddbfb2ac29c7082990bcaedaf40161ca9

branch: merged tutorial: ready ci: success
Open CI Details

Continuous Integration

CI: success

Compact status on the review page, with full logs on the CI page.

Open CI Details

Latest run #36 success

2 passed

head dd5fda25dc5411fab258dacdb9c3fafcac6a99d7 · queued 2026-03-24 14:17:40 · 2 lane(s)

queued 10s · ran 29s

check-notifications · success check-agent-contracts · success

Summary

This branch polishes the branch review detail page in pika-news by making three targeted changes: (1) the diff panel is moved out of the constrained review-layout grid and placed below it so it can use the full page width, (2) the tutorial error messaging is improved to clearly distinguish between a global summary-generator health warning and a per-branch generation failure, and (3) two new tests are added to assert both the updated warning copy and the new DOM ordering of the diff section relative to the review layout.

Tutorial Steps

Improve global health warning copy for the summary generator

Intent: Make the page-level health notice clearly describe a forge-wide generator problem rather than a vague error, so users can distinguish it from a branch-specific failure.

Affected files: crates/pika-news/src/web.rs

Evidence
@@ -956,7 +956,7 @@ fn branch_page_notices(state: &AppState) -> Vec<PageNoticeView> {
-            "Summary generation hit an error. New tutorial updates may be delayed until it recovers.",
+            "The summary generator is unhealthy. New tutorials across the forge may be delayed until it recovers.",

In branch_page_notices (crates/pika-news/src/web.rs:959), the warning string shown when the summary-generator health check reports an error state is reworded.

The old message — "Summary generation hit an error" — was ambiguous about scope. The new message — "The summary generator is unhealthy. New tutorials across the forge may be delayed until it recovers." — makes it explicit that this is a system-wide condition, not something specific to the branch being viewed.

Rename the per-branch error heading and add a failure-specific summary message

Intent: Give users a clear, branch-scoped signal when tutorial generation has failed for their specific branch, with distinct heading text and a contextual explanation in the summary section.

Affected files: crates/pika-news/templates/detail.html

Evidence
@@ -309,7 +312,7 @@
-            <h2>Generation Error</h2>
+            <h2>Branch Tutorial Generation Failed</h2>
@@ -351,7 +354,11 @@
+              {% if tutorial_status == "failed" %}
+              <p>This branch tutorial is unavailable because generation failed. Check the branch tutorial error above for details.</p>
+              {% else %}
               <p>Summary is not ready yet. Refresh shortly.</p>
+              {% endif %}

Two template changes in crates/pika-news/templates/detail.html sharpen the per-branch failure experience:

  1. Error heading (detail.html:314): Renamed from the generic "Generation Error" to "Branch Tutorial Generation Failed", which pairs well with the global warning without overlapping in meaning.

  2. Summary section (detail.html:357-361): When tutorial_status equals "failed", the summary panel now shows "This branch tutorial is unavailable because generation failed." instead of the optimistic "Summary is not ready yet. Refresh shortly." message, which would be misleading after a terminal failure.

Move the diff panel below the review layout for full-width display

Intent: Allow the diff viewer to use the full page width instead of being constrained inside the two-column review-layout grid, improving readability for wide diffs.

Affected files: crates/pika-news/templates/detail.html

Evidence
@@ -192,6 +192,9 @@
+      .diff-row {
+        margin-top: 1.25rem;
+      }
@@ -366,26 +373,27 @@
-            {% match diff_json %}
-            {% when Some with (json_str) %}
-            <section class="panel diff-panel">
+      {% match diff_json %}
+      {% when Some with (json_str) %}
+      <section class="panel diff-panel diff-row">
@@ -366,26 +373,27 @@
-            </section>
-            {% endmatch %}
           </div>
         </div>
       </div>
+
+      {% match diff_json %}

The diff panel ({% match diff_json %} block) is lifted out of the .review-layout > .review-main column and placed after the closing </div> tags of the review layout, directly inside <main>.

This is a structural change to crates/pika-news/templates/detail.html:373-399:

  • The <section> gains the additional CSS class diff-row.
  • A new .diff-row { margin-top: 1.25rem; } rule (detail.html:194-196) provides vertical spacing between the review grid and the now-full-width diff panel.
  • The diff toggle buttons and #diff-container markup are otherwise unchanged.

Because the diff section is no longer nested inside the constrained grid column, side-by-side diffs can expand to the viewport width, which is where they benefit most from extra space.

Add test: global generator health warning vs. branch-specific failure

Intent: Verify that the rendered detail page can simultaneously show the forge-wide generator-unhealthy banner and the per-branch failure heading and message without one suppressing the other.

Affected files: crates/pika-news/src/web.rs

Evidence
@@ -6822,6 +6823,113 @@
+    #[test]
+    fn branch_detail_distinguishes_global_generator_health_from_branch_failure() {
@@ -6822,6 +6823,113 @@
+        assert!(rendered.contains("The summary generator is unhealthy."));
+        assert!(rendered.contains("Branch Tutorial Generation Failed"));
+        assert!(rendered.contains("This branch tutorial is unavailable because generation failed."));

A new test at crates/pika-news/src/web.rs:6825 (branch_detail_distinguishes_global_generator_health_from_branch_failure) does the following:

  1. Creates a branch and marks its tutorial generation as failed via mark_branch_generation_failed.
  2. Renders the detail template with an injected PageNoticeView carrying the global health warning.
  3. Asserts that all three distinct messages appear in the HTML output:
    • The global banner: "The summary generator is unhealthy."
    • The branch error heading: "Branch Tutorial Generation Failed"
    • The summary fallback: "This branch tutorial is unavailable because generation failed."

This ensures the two layers of communication (system-wide vs. branch-specific) coexist correctly.

To call render_detail_template_with_notices and construct PageNoticeView in tests, the test module's import block is updated (web.rs:4579-4583) to bring both symbols into scope.

Add test: diff panel appears after review layout in DOM order

Intent: Guard the new full-width diff layout by asserting that the diff section's position in the rendered HTML comes after the review-layout container.

Affected files: crates/pika-news/src/web.rs

Evidence
@@ -6822,6 +6823,113 @@
+    #[test]
+    fn branch_detail_places_diff_after_review_layout() {
@@ -6822,6 +6823,113 @@
+        let review_layout = rendered
+            .find("class=\"review-layout\"")
+            .expect("review layout");
+        let diff_row = rendered
+            .find("class=\"panel diff-panel diff-row\"")
+            .expect("diff row");
+        assert!(diff_row > review_layout);

A second new test at crates/pika-news/src/web.rs:6880 (branch_detail_places_diff_after_review_layout) validates the structural change:

  1. Creates a branch and marks it ready with a minimal tutorial JSON payload and a small diff string.
  2. Renders the detail template via render_detail_template.
  3. Uses str::find to locate the byte offsets of class="review-layout" and class="panel diff-panel diff-row" in the rendered HTML.
  4. Asserts diff_row > review_layout, confirming the diff section appears later in the DOM than the review grid.

This is a lightweight structural regression test that will fail if someone accidentally moves the diff panel back inside the review layout.

Diff