fix(server): let the review write its own opening, and read as one message

A review comment opened with one of a small set of server-authored lines —
"Nice work …" for a clean change, "Worth keeping: …" for a strength beside a
problem — chosen from a curated slug-to-phrase map. The same sentence opened
every review that fell in the same bucket, and a change with a critical finding
could still open on praise.

The opening now comes from the review itself. A report_summary tool carries one
or two sentences out in the feedback envelope as `lead`, and the note opens on
them. Nothing stands in for a missing one: with no lead the note opens on its
first finding.

The note also stops working against itself. It no longer states its issue count
twice, a strength and its next step no longer run together unpunctuated, the
footer no longer carries the run's duration, and a defect found by opening a
file is quoted from the diff so the note lands beside the code.

The opening is model prose in the position a reader trusts most, so it is held
to a narrower contract than a finding body. The runner sends an over-long lead
back to be rewritten rather than cutting it mid-word; the parser takes it only
when it is a string; the composer scrubs it, clamps it at a sentence boundary,
and drops it whole if it reaches for markup that could restructure the comment
around it, for a link, or for a verdict on merging — which the composer is never
told and so could only invent. That last one was previously guaranteed by the
fixed phrasing rather than checked, and a two-sentence orientation has no
legitimate use for any of them.

Exactly one comment may open on the lead. The auto-posted note is composed
first and takes it; an approved proposal takes it only when there is no such
note, which is the all-approval workspace where the proposal is the only
comment the developer sees.

Code fences are now balanced at egress rather than in the composer, because the
length cut that runs there can reopen a block that was balanced upstream, and
everything below an open fence renders as code — including the AI-generated
disclosure the comment carries. Counting fences was not enough either: a block
ends only on a fence of the same character and at least the opening length
carrying no info string, so a quoted block inside a quote left the count even
and the document broken. Quoted evidence is now wrapped in a fence that outruns
any run of backticks inside it.

Two holes in that egress sanitizer went with it: an unterminated comment opener
matched nothing and reached the provider intact, and mentions were escaped only
after bracket-like characters, so `cc,@name` shipped live.

Approved proposals now go through that sanitizer, which they were bypassing
entirely, and are suppressed rather than posted when they sanitize to nothing —
the provider rejects an empty comment, and that exception would escape the
listener and strand the approval with nothing to retry it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LK2yxSEfNL5Q5xwbAqAMWr

Author: FelixTJDietrich

.changeset/review-comments-read-better.md

@@ -0,0 +1,5 @@
+---
+"hephaestus": patch
+---
+
+Review comments no longer repeat their own issue count in the opening line, no longer run a strength and its next step together into one unpunctuated sentence, and no longer report how long the run took.

.changeset/reviews-land-beside-the-code.md

@@ -0,0 +1,5 @@
+---
+"hephaestus": patch
+---
+
+A problem the review found by opening a file now arrives as a comment on the changed line, rather than as a paragraph at the bottom of the merge request.

.changeset/reviews-open-in-their-own-words.md

@@ -0,0 +1,5 @@
+---
+"hephaestus": patch
+---
+
+A review comment now opens with a sentence written about your change, instead of one of a handful of fixed lines that read the same on every review — including on reviews that opened with praise ahead of a serious problem. When the review has nothing worth opening on, it opens on its first finding.

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/ApprovedFeedbackDeliveryListener.java

@@ -95,8 +95,19 @@ public void deliver(ApprovedFeedbackReadyEvent event) {
             return;
         }
         if (existing.kind() == ExistingDeliveryLookup.Kind.ABSENT) {
+            String sanitized = PullRequestCommentPoster.sanitize(feedback.getBody());
+            // A provider rejects an empty comment, and the resulting exception would escape this listener and
+            // strand the approval in PREPARED with nothing to retry it.
+            if (sanitized.isBlank()) {
+                feedbackRepository.markApprovedSuppressed(
+                    event.workspaceId(),
+                    feedback.getId(),
+                    FeedbackSuppressionReason.EMPTY_AFTER_SANITIZE.name()
+                );
+                return;
+            }
             try {
-                commentPoster.postApprovedProposal(job, feedback.getId(), feedback.getBody());
+                commentPoster.postApprovedProposal(job, feedback.getId(), sanitized);
             } catch (JobDeliverySuppressedException exception) {
                 feedbackRepository.markApprovedSuppressed(
                     event.workspaceId(),

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/DeliveryComposer.java

@@ -260,12 +260,17 @@ public void deliver(AgentJob job) {
         );
         Map<String, String> why = practiceCatalogInjector.whyBySlug(job.getWorkspace(), ArtifactKinds.ISSUE);
         List<ComposedFeedbackUnit> units = compositionResultParser.parse(job.getOutput(), FeedbackChannel.IN_CONTEXT);
+        String lead = compositionResultParser.lead(job.getOutput());
+        var note = DeliveryComposer.compose(loudEnough, ArtifactKinds.ISSUE, why, null, units, lead);
+        // An approved proposal posts as its own comment, so only one of the two may open on the lead.
+        // The auto-posted note is composed first and takes it; the proposal takes it only when there is
+        // no such note, which is the all-approval workspace where the proposal is the only comment.
         feedbackLedgerRecorder.recordProposal(
             job,
-            DeliveryComposer.compose(proposals, ArtifactKinds.ISSUE, why, null, units),
+            DeliveryComposer.compose(proposals, ArtifactKinds.ISSUE, why, null, units, note == null ? lead : null),
             proposals
         );
-        postIssueNote(job, DeliveryComposer.compose(loudEnough, ArtifactKinds.ISSUE, why, null, units));
+        postIssueNote(job, note);
     }
 
     private PracticeDetectionResultParser.ValidatedObservation validated(Observation observation) {

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/IssueReviewHandler.java

@@ -2,7 +2,6 @@
 
 import de.tum.cit.aet.hephaestus.agent.job.AgentJob;
 import de.tum.cit.aet.hephaestus.config.ApplicationProperties;
-import java.time.Duration;
 import org.jspecify.annotations.Nullable;
 import org.springframework.stereotype.Component;
 import org.springframework.web.util.HtmlUtils;
@@ -56,12 +55,7 @@ private static void appendMetadataFooter(StringBuilder sb, AgentJob job) {
         if (modelName != null && !modelName.isBlank()) {
             sb.append(" · ").append(HtmlUtils.htmlEscape(modelName));
         }
-        if (job.getStartedAt() != null && job.getCompletedAt() != null) {
-            sb.append(" · ").append(formatDuration(Duration.between(job.getStartedAt(), job.getCompletedAt())));
-        }
-
-        sb.append("</sub>\n");
-        sb.append("<sub>AI-generated feedback can be inaccurate. React with 👍 or 👎 to give feedback.</sub>\n");
+        sb.append(" &middot; AI-generated and can be inaccurate. React with 👍 or 👎 to give feedback.</sub>\n");
     }
 
     @Nullable
@@ -72,14 +66,4 @@ private static String snapshotModelName(@Nullable JsonNode configSnapshot) {
         JsonNode model = configSnapshot.path("upstreamModelId");
         return model.isString() ? model.asString() : null;
     }
-
-    private static String formatDuration(Duration duration) {
-        long totalSeconds = Math.max(0, duration.toSeconds());
-        if (totalSeconds < 60) {
-            return totalSeconds + "s";
-        }
-        long minutes = totalSeconds / 60;
-        long seconds = totalSeconds % 60;
-        return minutes + "m " + seconds + "s";
-    }
 }

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/PracticeFeedbackCommentFormatter.java

@@ -41,7 +41,7 @@ class PullRequestCommentPoster {
      *  Lookbehind covers start-of-line, whitespace, punctuation, and markdown formatting chars
      *  ({@code * _ ~ > | -}) to prevent bypass via {@code *@user*}, {@code >@user}, or {@code - @user}. */
     private static final Pattern AT_MENTION = Pattern.compile(
-        "(?<=^|[\\s(\\[\"'*_~>|#!+={}\\-])@([a-zA-Z0-9][-a-zA-Z0-9._]*)",
+        "(?<=^|[\\s(\\[\"'*_~>|#!+={}\\-,.:;/)])@([a-zA-Z0-9][-a-zA-Z0-9._]*)",
         Pattern.MULTILINE
     );
 
@@ -406,6 +406,8 @@ static String sanitize(String raw) {
         result = result.replace("\r\n", "\n").replace("\r", "\n");
         result = INVISIBLE_CHARS.matcher(result).replaceAll("");
         result = HTML_COMMENT.matcher(result).replaceAll("");
+        // An unterminated opener matches nothing above, and runs to end of document in both renderers.
+        result = result.replace("<!--", "");
 
         // Autolinks become plain links first, so the tag stripping below does not eat them.
         result = AUTOLINK.matcher(result).replaceAll("$1");
@@ -438,13 +440,58 @@ static String sanitize(String raw) {
         result = EXCESSIVE_NEWLINES.matcher(result).replaceAll("\n\n");
 
         result = result.strip();
-        if (result.length() > MAX_BODY_LENGTH) {
-            result = result.substring(0, MAX_BODY_LENGTH) + "\n\n[... truncated — comment exceeded length limit]";
+        boolean truncated = result.length() > MAX_BODY_LENGTH;
+        if (truncated) {
+            result = result.substring(0, MAX_BODY_LENGTH);
+        }
+        // Last, because the cut above can reopen a block that was balanced upstream, and because everything
+        // below an open fence renders as code — including the AI-generated disclosure this comment carries.
+        result = balanceCodeFences(result);
+        if (truncated) {
+            result += "\n\n[... truncated — comment exceeded length limit]";
         }
 
         return result;
     }
 
+    /**
+     * Closes a fenced block the body leaves open. Counting fences is not enough: CommonMark ends a block
+     * only on a fence of the same character and at least the opening length that carries no info string, so
+     * an inner fence is content, a `~~~` is a fence, and an indented line is not one.
+     */
+    static String balanceCodeFences(String text) {
+        char openChar = 0;
+        int openLength = 0;
+        for (String line : text.split("\n", -1)) {
+            String stripped = line.stripLeading();
+            if (line.length() - stripped.length() >= 4 || stripped.isEmpty()) {
+                continue;
+            }
+            char marker = stripped.charAt(0);
+            if (marker != '`' && marker != '~') {
+                continue;
+            }
+            int run = 0;
+            while (run < stripped.length() && stripped.charAt(run) == marker) {
+                run++;
+            }
+            if (run < 3) {
+                continue;
+            }
+            String info = stripped.substring(run);
+            if (openLength == 0) {
+                if (marker == '`' && info.indexOf('`') >= 0) {
+                    continue;
+                }
+                openChar = marker;
+                openLength = run;
+            } else if (marker == openChar && run >= openLength && info.isBlank()) {
+                openLength = 0;
+            }
+        }
+        return openLength == 0 ? text : text + "\n" + String.valueOf(openChar).repeat(openLength);
+    }
+
     static String requireMetadataText(@Nullable JsonNode metadata, String field) {
         if (metadata == null) {
             throw new JobDeliveryException("Missing required metadata field: " + field);

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/PullRequestCommentPoster.java

@@ -336,15 +336,26 @@ private void deliverAdmitted(AgentJob job) {
             .evaluate(job, loudEnough)
             .deliverable();
         List<ComposedFeedbackUnit> units = compositionResultParser.parse(job.getOutput(), FeedbackChannel.IN_CONTEXT);
+        String lead = compositionResultParser.lead(job.getOutput());
         Map<String, String> why = practiceCatalogInjector.whyBySlug(job.getWorkspace(), ArtifactKinds.PULL_REQUEST);
+        var content = DeliveryComposer.compose(deliverable, ArtifactKinds.PULL_REQUEST, why, unifiedDiff, units, lead);
+        // An approved proposal posts as its own comment, so only one of the two may open on the lead.
+        // The auto-posted note is composed first and takes it; the proposal takes it only when there is
+        // no such note, which is the all-approval workspace where the proposal is the only comment.
         feedbackService.recordProposal(
             job,
-            DeliveryComposer.compose(proposals, ArtifactKinds.PULL_REQUEST, why, unifiedDiff, units),
+            DeliveryComposer.compose(
+                proposals,
+                ArtifactKinds.PULL_REQUEST,
+                why,
+                unifiedDiff,
+                units,
+                content == null ? lead : null
+            ),
             proposals
         );
-        var content = DeliveryComposer.compose(deliverable, ArtifactKinds.PULL_REQUEST, why, unifiedDiff, units);
         feedbackService.deliverFeedback(job, content, delivered ->
-            DeliveryComposer.recomposeMrNote(deliverable, ArtifactKinds.PULL_REQUEST, why, delivered, units)
+            DeliveryComposer.recomposeMrNote(deliverable, ArtifactKinds.PULL_REQUEST, why, delivered, units, lead)
         );
     }
 

server/src/main/java/de/tum/cit/aet/hephaestus/agent/handler/PullRequestReviewHandler.java

@@ -30,6 +30,9 @@ public class FeedbackCompositionResultParser {
 
     private static final int MAX_PRACTICE_SLUG_LENGTH = 128;
 
+    /** A ceiling on the payload, not on the opening: the composer clamps the lead to its own budget. */
+    private static final int MAX_LEAD_LENGTH = 4_000;
+
     public List<ComposedFeedbackUnit> parse(@Nullable JsonNode jobOutput) {
         if (jobOutput == null || !jobOutput.isObject()) {
             return List.of();
@@ -270,6 +273,12 @@ public List<ComposedFeedbackUnit> parse(@Nullable JsonNode jobOutput, FeedbackCh
         );
     }
 
+    /** How the review opens, in the composer's words; null when it wrote none. */
+    @Nullable
+    public String lead(@Nullable JsonNode jobOutput) {
+        return jobOutput == null ? null : text(jobOutput.path("feedback"), "lead", MAX_LEAD_LENGTH);
+    }
+
     private static Map<String, StagedObservation> readObservations(@Nullable JsonNode node) {
         Map<String, StagedObservation> observations = new LinkedHashMap<>();
         if (node == null || !node.isArray()) {