feat(titles): terse 'keyword -> gist' titles, keep user first-line hints, CAS name updates#27
Merged
Merged
Conversation
…nts, CAS name updates
A pasted line containing ' -> ' (e.g. a log line) could be stored verbatim as the workspace name with no length bound. Arrow hints now only win at a plausible title length (<= 100 chars, matching the frontend's first-line name cap); longer lines fall through to the model path, which caps titles at 48 chars.
Simplify pass: compute the first line's char count once instead of two O(n) scans, and remove the no-op .trim() — split_whitespace/join leaves no leading whitespace and trim_end_matches already covers the trailing space a mid-cut can leave.
…odel paths The hint and Haiku paths normalized candidate titles with drifting rules (hint trimmed only trailing periods and kept raw whitespace; the model path collapsed whitespace and trimmed all sentence punctuation), so the same first line could be stored differently depending on which path won. Extract canonicalize_title() — whitespace collapse, unicode-arrow normalization, char cap, trailing-punctuation trim — and call it from both; length policy stays per-path. Also guard against a hint line that canonicalizes to empty (e.g. '.').
Review findings from the codex xhigh pass:
- a 48-char cap cut inside ' -> ' left a dangling arrow fragment; strip
' ->' / ' -' suffixes after the cap and re-trim
- '#'-stripping ate issue-ref prefixes ('#27 -> x' lost its hash); only
strip a Markdown ATX heading marker ('#' run followed by whitespace)
- unicode ellipsis '…' now rejects a hint line like its ASCII '...' twin
- unicode arrows are normalized BEFORE the hint length check, so a line
that grows past the cap once ' → ' expands to ' -> ' is rejected, not
truncated; adjacent arrows ('a → → b') all normalize via a stable loop
- an un-sluggable hint (all CJK/emoji/punctuation) produced a degenerate
'<prefix>/<uuid>-' branch; the rename is now skipped (heuristic branch
kept) while the user's title still lands, mirroring the model path's
empty-slug bail-out
…ATX strip
Final review-battery findings:
- the dangling-arrow strip was single-pass, so chained tails ('a -> ->',
cap cuts through 'x -> -> b') kept a fragment; trim punctuation and
arrow fragments to a fixpoint (each pass strictly shortens, so it
terminates)
- the arrow-hint length bound was measured before whitespace collapse,
rejecting a line whose stored form is short ('bp<50sp>-><50sp>dogfood');
acceptance now measures the collapsed+normalized form it would store
- heading stripping accepted 7+ leading hashes, which CommonMark treats
as content, not an ATX heading; only strip 1-6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Workspace titles were generated with a hardcoded PR-title prompt ("imperative summary, at most 6 words") — exactly the "Fix bug where..." style we do not want. Worse, the user's deliberate first-line title was destroyed: the frontend always sends the prompt's first line as the workspace
name, and the background Haiku job then unconditionally overwrote it (no compare-and-set on name, unlike the branch which already had one). Manual renames during the ~5-20s Haiku window and MCP-provided names were clobbered too.What changed (backend only)
keyword -> gisttitles. Replaced the prompt incrates/executors/src/title_gen.rswith one that asks for<keyword> -> <gist>(2-5 word gist, 40-char cap, lowercase except proper nouns, terse noun fragments), with Miguel's good/bad examples and a first-line-reuse rule.TITLE_MAX_CHARS72 -> 48; unicode→normalized to->; trailing punctuation trimmed after the char cap.first_line_title_hint(): a first line containing->always wins; a short first line (<=8 words, <=48 chars) with a body also wins; trailing:/,/...rejects. When a hint exists we skip the Haiku call entirely — keep the hint verbatim as the name and derive the branch deterministically from it (instant, no model latency).Workspace::update_name_if_unchangedCAS (NULL-safe SQLiteIS, modeled on the existingupdate_branch_name_if_unchanged) replaces the unconditionalWorkspace::update.expected_oldis the pre-spawn workspace-row snapshot name, so a manual rename or an MCP-provided name that lands in the window survives. The MCPstart_workspacepath is protected by this CAS alone.->but never reach a branch name raw — both paths route throughslugify/git_branch_id_with_len, pinned by new unit tests ("bp -> runflow dogfood"->"bp-runflow-dogfood").How verified
cargo test -p executors -p utils— 87 tests pass, including the newfirst_line_title_hintaccept/reject matrix, arrow-in-title / 48-char-cap / trailing-period parse tests, and the slugify/git_branch_id_with_lenarrow-degradation tests.cargo check -p db -p servercompiles clean withSQLX_OFFLINE=true;pnpm run prepare-dbregenerated the offline metadata for the new query (committed undercrates/db/.sqlx).cargo fmtapplied to touched crates.claude-code --model haiku— output parsed as JSON and matchedkeyword -> gistunder 40 chars ({"title":"email -> bp off customer.io", ...}).Risks
keyword ->— this matches the "user hints win" decision; drop theshort_enough && has_bodyclause if restyle-always-except-arrow is wanted.ship-it battery
Full review battery run on this PR (base commit
519cb948, final commitf6a1ea02).Council round 1 (mixed engine: 4 Codex personas + 1 Claude cross-check seat, all completed). No P0. Applied: reject overlong arrow first lines (>100 chars) as title hints so a pasted log line containing
->can never be stored verbatim as a workspace name (a972bf73). Rejected with reasons: MCP explicit-name suppression (locked decision: CAS-only protection;name.is_some()cannot discriminate user intent since the frontend always sends it — flagged for a future provenance field); coupling the branch rename to the name-CAS outcome (independent CAS semantics by design, pre-dating this PR); arrow+trailing-colon acceptance and prompt rewording (heuristic and prompt text are locked as specified); git-refs-before-DB-CAS TOCTOU insiderename_workspace_branch(pre-existing shared infra from #17, unchanged here — noted as known debt: a background rename can make a concurrently issued manual branch rename fail with a visible error).Simplify pass (4 parallel cleanup agents: reuse/simplification/efficiency/altitude). Applied: hoist duplicate char count, drop a no-op
trim(f77c9dca); folded both title paths into onecanonicalize_title()so the hint and Haiku paths can never drift in stored form (e036d514). Skipped with reasons: replacing the pre-existing localslugifywithgit_branch_id_with_len(drive-by refactor outside the diff; the dual-layer test pinning is deliberate defense in depth).Codex + xhigh review (Codex CLI xhigh background review + 5 parallel correctness finder angles + conventions check). Applied (
eacbdd11): strip dangling arrow fragments after the 48-char cap; ATX-only heading strip so#27 -> xkeeps its hash; unicode ellipsis…rejects like...; unicode arrows normalized before the hint length check; skip the generated branch rename when the seed slugs to empty (all-CJK/emoji/punctuation hints kept as the name, heuristic branch preserved) — each pinned by unit tests.Council round 2 (mixed, on the updated diff): unanimous APPROVE, zero P0/P1 across all 5 seats. The Claude seat additionally fuzz-verified the fixes (200k hostile inputs: no panics, loop-termination proof, byte-slice boundary proof,
ends_with('-')guard exactness) and surfaced one cosmetic P2 — the tail-trim was single-pass; fixed with a fixpoint loop (f6a1ea02).CodeRabbit: substituted. The CodeRabbit CLI fails deterministically in this non-interactive environment ("Non-interactive environment detected. Use --api-key"); per the run conventions a Codex xhigh backstop reviewed the final diff instead. It reported no P0/P1; its two P2s (hint length measured before whitespace collapse; 7+-hash runs stripped as headings contrary to CommonMark) are fixed in
f6a1ea02. Zero actionable findings remain.Verification:
cargo test -p executors -p utils -p dbgreen after every fix batch (91 executors + 4 utils tests at HEAD, 24 of them for title_gen/hint edge cases);cargo clippyclean on touched crates;cargo fmtapplied; no new SQLx queries beyond the round-0.sqlxentry (already committed). Not verified live: end-to-end workspace creation against a running backend (unchanged from the PR's original honest caveat).