Skip to content

Commit 7ee16b9

Browse files
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. Three constraints that the fixed phrasing satisfied by construction move into the composer prompt, which is where that judgement now happens: the opening implies no verdict on merging, does not lead with praise above a CRITICAL or MAJOR finding, and does not restate what follows. 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. Nothing on the comment now quotes the practice catalogue. The workspace's own paragraph about why a practice matters used to follow every note, identical on every review that touched that practice and written about the practice rather than about the change. It is gone from findings, from the notes on the diff and from the all-strengths note, and the prompt says plainly that a practice decides what is raised and never appears in the wording. The "On the diff:" label goes with it; the review's opening already says so in its own words. The lead is model-authored text on a public comment, so it is bounded rather than trusted: the runner sends an over-long lead back to be rewritten instead of cutting it mid-word, the parser takes it only when it is a string, the composer re-applies the budget at a sentence boundary and scrubs it, and an unbalanced code fence anywhere in an assembled note is closed so one bad fence cannot render the whole review as a code block. A lead is given to the note that posts first, so an approved proposal and an auto-posted summary cannot open two comments on one change with the same sentence. report_summary is admitted on the same gate as report_feedback, so the detection stage cannot write the opening. Approved proposals now go through the same egress sanitizer as every other comment this posts, which they were bypassing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LK2yxSEfNL5Q5xwbAqAMWr
1 parent dd328ec commit 7ee16b9

36 files changed

Lines changed: 1055 additions & 1228 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"hephaestus": patch
3+
---
4+
5+
A review comment now stays as it was written. Where a later look at the same change used to rewrite the original comment in place — and quietly demote anything already answered on the diff — it now leaves a new comment beside the old one, the way a person would. The comment also stops repeating the notes that sit on the diff: those live on the lines they are about, and anything that could not be placed on a line falls back into the comment rather than disappearing between the two.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"hephaestus": patch
3+
---
4+
5+
Notes prepared for the mentor now say where a point has already been put to the developer and whether anything has moved without help, so a conversation does not repeat feedback they have already had twice. Notes written before this carry no such record, which reads as nothing having been said rather than as nothing to say.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"hephaestus": patch
3+
---
4+
5+
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.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"hephaestus": patch
3+
---
4+
5+
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.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"hephaestus": patch
3+
---
4+
5+
A review comment now leads with its most serious finding and the one edit that fixes it, instead of leading with whichever finding happened to have no line number attached. Each finding says what to do before it says why it matters, and the reasoning is one sentence rather than the same paragraph on every review that touches the practice.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"hephaestus": patch
3+
---
4+
5+
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.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"hephaestus": patch
3+
---
4+
5+
Review comments no longer append the workspace's own wording about a practice to each note. That paragraph was identical on every review that touched the practice, and it was about the practice rather than about the change in front of you. The practice still decides what gets raised; it just doesn't get quoted back.

server/src/main/java/de/tum/cit/aet/hephaestus/agent/context/providers/mentor/PreparedConversationFeedbackContentSource.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,11 @@ private static void writeNotes(ObjectNode node, String body) {
146146
notes.put("capability", brief.capability());
147147
notes.put("evidenceSummary", brief.evidenceSummary());
148148
notes.put("inConversationSignal", brief.inConversationSignal());
149+
// Absent on a brief written before the field existed, and absent when nothing has been put to them
150+
// yet. Either way the mentor is told nothing rather than told there is nothing.
151+
if (brief.alreadySaid() != null) {
152+
notes.put("alreadySaid", brief.alreadySaid());
153+
}
149154
}
150155

151156
/**

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,19 @@ public void deliver(ApprovedFeedbackReadyEvent event) {
9595
return;
9696
}
9797
if (existing.kind() == ExistingDeliveryLookup.Kind.ABSENT) {
98+
String sanitized = PullRequestCommentPoster.sanitize(feedback.getBody());
99+
// A provider rejects an empty comment, and the resulting exception would escape this listener and
100+
// strand the approval in PREPARED with nothing to retry it.
101+
if (sanitized.isBlank()) {
102+
feedbackRepository.markApprovedSuppressed(
103+
event.workspaceId(),
104+
feedback.getId(),
105+
FeedbackSuppressionReason.EMPTY_AFTER_SANITIZE.name()
106+
);
107+
return;
108+
}
98109
try {
99-
commentPoster.postApprovedProposal(job, feedback.getId(), feedback.getBody());
110+
commentPoster.postApprovedProposal(job, feedback.getId(), sanitized);
100111
} catch (JobDeliverySuppressedException exception) {
101112
feedbackRepository.markApprovedSuppressed(
102113
event.workspaceId(),

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

Lines changed: 183 additions & 296 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)