Skip to content

feat(mark): add AI-generated one-sentence review titles#349

Merged
matt2e merged 5 commits intomainfrom
one-sentence-review
Mar 5, 2026
Merged

feat(mark): add AI-generated one-sentence review titles#349
matt2e merged 5 commits intomainfrom
one-sentence-review

Conversation

@matt2e
Copy link
Contributor

@matt2e matt2e commented Mar 5, 2026

Summary

  • Add a title column to reviews and a review-title fenced block to the AI review prompt, so each code review gets an AI-generated one-sentence title reflecting confidence level
  • Extract and persist the review title from assistant output in session_runner, alongside existing comment extraction
  • Display the review title in the branch timeline UI instead of the generic "Code Review" label
  • Includes schema migration (v17→v18), updated SQL queries, Rust model changes, and frontend type/component updates

Test plan

  • Existing unit tests updated for new prompt format
  • New unit tests for extract_review_title (simple, whitespace trimming, missing, empty, alongside comments)
  • Verify schema migration on existing database (v17→v18 adds title column)
  • Manually trigger a review session and confirm the title appears in the timeline

🤖 Generated with Claude Code

matt2e and others added 3 commits March 5, 2026 16:27
Ask the review agent for a single-sentence title (max 15 words) that
conveys overall confidence in the changes. The title is returned in a
```review-title fenced block alongside the existing review-comments
block.

Backend changes:
- Add nullable `title` column to the `reviews` table (schema v18)
- Add `title` field to the Review model and ReviewTimelineItem
- Extract the review title from agent output in session_runner and
  persist it via a new `update_review_title` store method
- Include the title in branch context timeline entries

Frontend changes:
- Add `title` to the ReviewTimelineItem TypeScript type
- Display the review title in the branch timeline, falling back to
  "Code Review" when no title is available
…scription

Update the code review prompt to explicitly instruct the LLM to focus
the review title on its confidence level rather than describing what the
changes do. This produces more useful at-a-glance signals in the branch
timeline.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop the "This title will be shown in the branch timeline" clause from
the review prompt — it leaks UI implementation details to the LLM
without improving output quality.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@matt2e matt2e requested review from baxen and wesbillman as code owners March 5, 2026 05:45
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0accc05f76

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +526 to +527
conn.execute_batch("ALTER TABLE reviews ADD COLUMN title TEXT DEFAULT NULL;")
.ok(); // Ignore error if column already exists (fresh DB)

Choose a reason for hiding this comment

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

P1 Badge Fail migration if adding reviews.title does not succeed

This migration swallows every ALTER TABLE error via .ok() and then unconditionally stamps schema_version to 18, so a non-duplicate failure (for example a locked/corrupt DB) leaves the DB marked as upgraded while still missing reviews.title. After that, the new SELECT ... title ... FROM reviews queries in store/reviews.rs will fail at runtime with no such column: title, effectively breaking review loading until a reset. The migration should only ignore the specific "duplicate column" case or abort without bumping the schema version.

Useful? React with 👍 / 👎.

Comment on lines +789 to +792
let start_pos = text.find(marker)?;
let block_start = start_pos + marker.len();
let content_start = block_start + text[block_start..].find('\n')? + 1;
let end_pos = find_closing_fence(&text[content_start..])?;

Choose a reason for hiding this comment

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

P2 Badge Parse beyond first review-title marker

extract_review_title only checks the first occurrence of ````review-title`` and returns immediately; if that first block is malformed/empty but a later assistant message contains a valid title block, the function still returns None and the review title is never persisted. Since `run_post_completion_hooks` concatenates all assistant messages and retries can produce multiple blocks, this causes lost titles in real sessions. Iterate through markers (like `extract_review_comments`) and pick a valid non-empty block.

Useful? React with 👍 / 👎.

matt2e and others added 2 commits March 5, 2026 17:12
…ction

When the LLM discusses the extraction logic in its preamble (e.g.
mentioning `"```review-title"` as a string), `text.find(marker)` matched
the mid-line mention instead of the actual fenced block. This caused the
entire preamble + fenced block content to be stored as the review title.

Fix by adding `find_opening_fence()` which requires the marker to appear
at column 0 (start of text or after `\n`), matching the same convention
used by `find_closing_fence()`. Applied to both title and comment
extraction.

Also suppress streaming hints when review fenced blocks are detected
(the surrounding text is just LLM preamble, not a useful status), and
instruct the LLM not to produce text outside the fenced blocks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop the "Do not include any text outside the two fenced blocks"
instruction and its corresponding test assertion, allowing the LLM to
produce preamble text if it wants to.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@matt2e matt2e merged commit d95387b into main Mar 5, 2026
4 checks passed
@matt2e matt2e deleted the one-sentence-review branch March 5, 2026 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant