[JacobStephens2]

Summary

A Review run passes the pull request's captured base commit (snapshot.base_head) to prompt::pull_request_review as the fixed point. The old helper wrapped every base in refs/remotes/origin/. That told reviewers to use refs/remotes/origin/<sha>, which git cannot resolve (fatal: Needed a single revision, exit 128).

-fn origin_base_ref(base) -> String
+fn fixed_point_revision(base) -> String
+  if base is 40 or 64 hex characters      # full commit object ID (SHA-1 / SHA-256)
+    return base
   return "refs/remotes/origin/{base}"     # Base branch, e.g. main

• The helper is renamed because it no longer always returns an origin ref (review finding).
• Base branches render exactly as before. base_branch_review_fixed_points_use_origin now asserts refs/remotes/origin/<base> literally. Before, it compared the prompt against the helper under test.
• The prompts page uses the placeholder <base>, so generated prompts don't change.

Evidence

Execution evidence

• Before (merge base 3beacb7f, with only the new test applied): cargo test --bin thirdshift prompt::tests::commit_review_fixed_points_name_the_commit

  thread 'prompt::tests::commit_review_fixed_points_name_the_commit' panicked at src/prompt.rs:1162:17:
  /thirdshift-code-review refs/remotes/origin/9ce638c79ce638c79ce638c79ce638c79ce638c7
  test result: FAILED. 0 passed; 1 failed
  

  After (this branch):

  cargo test --bin thirdshift prompt::tests   -> test result: ok. 8 passed; 0 failed
  cargo test prompts_page                     -> test result: ok. 14 passed; 0 failed
  scripts/affected-tests main                 -> PASS cargo test (895.90s), full suite
  

Confirmed cause: origin_base_ref (src/prompt.rs) always formatted refs/remotes/origin/{base}. ReviewProcess (src/processes/review.rs:349-351) passes the commit SHA snapshot.base_head (GitHub's base.sha) as base.

Visual evidence

Not applicable. This change has no rendered UI.

Merge Danger

Door: two-way

Blast Radius: narrow

Only prompt text changes, and only when the base is a full commit ID. Today that happens only for the Review run's fixed point.

Review coverage

Both axes read every changed file (src/prompt.rs). No generated output changed. Standards raised 3 judgement calls; Spec raised 1 test-strength finding.

Addressed:

• Standards, Mysterious Name: origin_base_ref is renamed fixed_point_revision and its doc comment rewritten.
• Spec, circular branch test: the Base branch test now asserts refs/remotes/origin/{base} literally.

Unaddressed findings

Standards

• Primitive Obsession: base: &str carries a Base branch or a commit. The helper tells them apart by shape, so a Base branch named exactly 40 or 64 hex characters renders bare.
  • I ran the finding's test, hex_named_base_branch_keeps_origin_ref (origin_base_ref(&"a".repeat(40))), with cargo test --bin thirdshift prompt::tests::hex_named_base_branch_keeps_origin_ref. It fails as the finding says: left: "aaaa…", right: "refs/remotes/origin/aaaa…".
  • Declined because the Spec makes this call. Review run prompt formats commit-SHA fixed point as invalid refs/remotes/origin/<sha> #2046 asks that "a full 40-character (or 64-character) hexadecimal commit object ID rather than a branch name" be rendered "directly without prefixing refs/remotes/origin/". It names origin_base_ref as the place that should "distinguish a raw commit SHA from a branch name".
  • Git also resolves a bare full hex object name before any ref of that name, so such a branch name is already ambiguous.
  • A FixedPoint type would change every prompt builder's signature. Review run prompt formats commit-SHA fixed point as invalid refs/remotes/origin/<sha> #2046's out-of-scope list excludes changes beyond the fixed-point reference.
• Duplicated Code: the full-commit-ID check also exists in src/evals/format.rs:289-297, src/github/advisories/description.rs:44 and src/scoring/origins.rs:257-259.
  • Declined. Those copies deliberately differ: commit_id accepts lowercase only, and is_commit accepts SHA-1 only.
  • Unifying them would change behaviour in modules Review run prompt formats commit-SHA fixed point as invalid refs/remotes/origin/<sha> #2046 does not touch, which is outside its scope.

Spec

None.

Closes #2046

Built with claude 2.1.314 · opus · default effort

Security review outcome

No unaddressed introduced findings.