diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index a1bb57d..f93ce0d 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -16,6 +16,38 @@ on: description: "Commit SHA to review (passed through from caller workflow_dispatch)." required: false type: string + review_model: + description: "Explicit model for analysis and optional native benchmark." + type: string + default: "gpt-5.6-terra" + review_effort: + description: "Reasoning effort: low, medium, high, xhigh, or max." + type: string + default: "high" + analysis_timeout_minutes: + description: "Timeout per analysis engine, between 1 and 30 minutes." + type: number + default: 12 + codex_version: + description: "Pinned stable CLI version; candidate versions can be tested in dry runs." + type: string + default: "0.149.0" + incremental: + description: "Review changes since the last compatible completed review." + type: boolean + default: true + review_bot_login: + description: "Trusted reviewer bot whose completion markers select incremental ranges." + type: string + default: "jitsu-code-review[bot]" + dry_run: + description: "Validate and archive results without publishing or recording coverage." + type: boolean + default: false + benchmark_native: + description: "Also run codex exec review for comparison; implies dry-run publication." + type: boolean + default: false secrets: OPENAI_API_KEY: required: true @@ -40,6 +72,38 @@ on: commit_sha: description: Commit SHA to review (leave blank when using PR number) required: false + review_model: + description: "Explicit model for analysis and optional native benchmark." + type: string + default: "gpt-5.6-terra" + review_effort: + description: "Reasoning effort: low, medium, high, xhigh, or max." + type: string + default: "high" + analysis_timeout_minutes: + description: "Timeout per analysis engine, between 1 and 30 minutes." + type: number + default: 12 + codex_version: + description: "Pinned stable CLI version; candidate versions can be tested in dry runs." + type: string + default: "0.149.0" + incremental: + description: "Review changes since the last compatible completed review." + type: boolean + default: true + review_bot_login: + description: "Trusted reviewer bot whose completion markers select incremental ranges." + type: string + default: "jitsu-code-review[bot]" + dry_run: + description: "Validate and archive results without publishing or recording coverage." + type: boolean + default: false + benchmark_native: + description: "Also run codex exec review for comparison; implies dry-run publication." + type: boolean + default: false env: REVIEW_INSTRUCTIONS: ${{ inputs.review_instructions || '' }} @@ -55,7 +119,7 @@ jobs: github.event_name != 'pull_request' || github.event.pull_request.draft == false runs-on: ubuntu-latest - timeout-minutes: 25 + timeout-minutes: 75 # Two engines at up to 30 minutes each, plus setup/artifacts. permissions: contents: read @@ -65,10 +129,35 @@ jobs: review_mode: ${{ steps.resolve-target.outputs.review_mode }} pr_number: ${{ steps.resolve-target.outputs.pr_number }} compare_head: ${{ steps.resolve-target.outputs.compare_head }} - compare_base: ${{ steps.prepare-context.outputs.compare_base }} + compare_base: ${{ steps.select-range.outputs.compare_base }} + full_base: ${{ steps.prepare-context.outputs.compare_base }} + review_profile: ${{ steps.select-range.outputs.review_profile }} subject_url: ${{ steps.resolve-target.outputs.subject_url }} + env: + REVIEW_MODEL: ${{ inputs.review_model || 'gpt-5.6-terra' }} + REVIEW_EFFORT: ${{ inputs.review_effort || 'high' }} + CODEX_VERSION: ${{ inputs.codex_version || '0.149.0' }} + REVIEW_BOT_LOGIN: ${{ inputs.review_bot_login || 'jitsu-code-review[bot]' }} + steps: + - name: "โš™๏ธ Validate execution settings" + env: + TIMEOUT_MINUTES: ${{ inputs.analysis_timeout_minutes }} + run: | + python3 - <<'PYTHON_EOF' + import os, re + if not re.fullmatch(r'[a-zA-Z0-9._-]+', os.environ['REVIEW_MODEL']): + raise ValueError('Expected an explicit model identifier') + if os.environ['REVIEW_EFFORT'] not in ('low', 'medium', 'high', 'xhigh', 'max'): + raise ValueError('Unsupported reasoning effort') + if not re.fullmatch(r'\d+\.\d+\.\d+', os.environ['CODEX_VERSION']): + raise ValueError('Expected a pinned stable CLI version') + timeout = float(os.environ['TIMEOUT_MINUTES'] or '12') + if not timeout.is_integer() or not 1 <= timeout <= 30: + raise ValueError('Timeout must be an integer from 1 to 30 minutes') + PYTHON_EOF + - name: "๐Ÿงญ Resolve review target" id: resolve-target env: @@ -244,7 +333,7 @@ jobs: cat > .github/workflows/ai-review.prompt.md << 'PROMPT_EOF' You are reviewing a GitHub change set for bugs, security issues, correctness problems, and user-visible regressions. - Read `.github/codex/context/review-target.txt` first. The checkout is the exact compare_head; review `git diff compare_base compare_head` using the recorded SHAs. For PRs, compare_base is the merge base, not the base branch tip. + Read `.github/codex/context/review-target.txt` first. The checkout is the exact compare_head; review `git diff compare_base compare_head` using the recorded SHAs. Read `.github/codex/context/range.json` for full versus incremental scope. An incremental range starts at the last compatible completed review, while full_base always records the PR merge base. Inspect earlier code as needed to validate new changes, without re-reviewing unchanged code. Context: - In PR mode, read `.github/codex/context/pr.json` for the title, description, and intentional decisions/tradeoffs. Read `review-threads.json` and `review-thread-comments.json` in that directory for previous findings and their explanations. @@ -276,10 +365,88 @@ jobs: Never use or mention "WIP". PROMPT_EOF + - name: "๐Ÿ“ Select review range" + id: select-range + if: ${{ steps.resolve-target.outputs.review_mode != 'skip' }} + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + REVIEW_MODE: ${{ steps.resolve-target.outputs.review_mode }} + PR_NUMBER: ${{ steps.resolve-target.outputs.pr_number }} + COMPARE_HEAD: ${{ steps.resolve-target.outputs.compare_head }} + FULL_BASE: ${{ steps.prepare-context.outputs.compare_base }} + INCREMENTAL: ${{ toJSON(inputs.incremental) != 'false' }} + run: | + python3 - <<'PYTHON_EOF' + import hashlib + import json + import os + from pathlib import Path + import re + import subprocess + + + def select_range(reviews, head, full_base, profile, bot, ancestor): + for review in reversed(reviews): + if (review.get('user') or {}).get('login') != bot or review.get('state') != 'APPROVED': + continue + for raw in re.findall(r'', review.get('body') or ''): + try: + coverage = json.loads(raw) + except ValueError: + continue + previous = coverage.get('head', '') if isinstance(coverage, dict) else '' + if (not re.fullmatch(r'[0-9a-f]{40}', previous) or coverage.get('version') != 1 + or previous != review.get('commit_id') or coverage.get('profile') != profile): + continue + if coverage.get('base') == full_base and previous != head and ancestor(previous, head): + return previous, 'incremental' + # Rewrites/base changes require a full review, not an older fallback marker. + return full_base, 'full' + return full_base, 'full' + + + def main(): + root = Path('.github/codex') + head, full_base = os.environ['COMPARE_HEAD'], os.environ['FULL_BASE'] + pr = json.loads((root / 'context/pr.json').read_text()) if os.environ['REVIEW_MODE'] == 'pr' else None + settings = {key: os.environ[key] for key in ('REVIEW_MODEL', 'REVIEW_EFFORT', 'CODEX_VERSION', 'REVIEW_BOT_LOGIN')} + material = {'settings': settings, 'pr': pr, + 'prompt': Path('.github/workflows/ai-review.prompt.md').read_text(), + 'additional': (root / 'context/additional-instructions.txt').read_text()} + # Head is range metadata, not review guidance: it must not invalidate every fixup. + if pr: + material['pr'] = {k: pr[k] for k in ('title', 'body')} + profile = hashlib.sha256(json.dumps(material, sort_keys=True).encode()).hexdigest() + reviews = [] + if os.environ['REVIEW_MODE'] == 'pr' and os.environ['INCREMENTAL'] == 'true': + raw = subprocess.check_output(['gh', 'api', f"repos/{os.environ['REPO']}/pulls/{os.environ['PR_NUMBER']}/reviews?per_page=100", + '--paginate', '--slurp'], text=True) + reviews = [review for page in json.loads(raw) for review in page] + def ancestor(old, new): + return subprocess.run(['git', 'merge-base', '--is-ancestor', old, new], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL).returncode == 0 + base, kind = select_range(reviews, head, full_base, profile, os.environ['REVIEW_BOT_LOGIN'], ancestor) + info = dict(compare_base=base, compare_head=head, full_base=full_base, range_kind=kind, review_profile=profile, **settings) + for folder in ('context', 'output'): + (root / folder / 'range.json').write_text(json.dumps(info, indent=2) + '\n') + target = root / 'context/review-target.txt' + target.write_text(re.sub(r'^compare_base:.*$', 'compare_base: ' + base, target.read_text(), flags=re.M)) + for flag, filename in [('--stat', 'diff-stat.txt'), ('--name-only', 'diff-files.txt')]: + (root / 'output' / filename).write_text(subprocess.check_output(['git', 'diff', flag, base, head], text=True)) + with open(os.environ['GITHUB_OUTPUT'], 'a') as output: + for key in ('compare_base', 'review_profile', 'range_kind'): + output.write(f'{key}={info[key]}\n') + + + if __name__ == '__main__': + main() + PYTHON_EOF + - name: "๐Ÿค– Run Codex analysis" if: ${{ steps.resolve-target.outputs.review_mode != 'skip' }} id: run-codex - timeout-minutes: 12 + timeout-minutes: ${{ inputs.analysis_timeout_minutes || 12 }} env: REVIEW_MODE: ${{ steps.resolve-target.outputs.review_mode }} REPO: ${{ github.repository }} @@ -296,12 +463,12 @@ jobs: output-file: .github/codex/output/review.json output-schema: >- {"type": "object", "additionalProperties": false, "required": ["version", "reviewed_sha", "completion_status", "summary", "findings"], "properties": {"version": {"type": "integer", "enum": [1]}, "reviewed_sha": {"type": "string"}, "completion_status": {"type": "string", "enum": ["complete", "incomplete"]}, "summary": {"type": "string"}, "findings": {"type": "array", "items": {"type": "object", "additionalProperties": false, "required": ["priority", "title", "body", "path", "side", "start_line", "end_line"], "properties": {"priority": {"type": "string", "enum": ["P0", "P1", "P2"]}, "title": {"type": "string"}, "body": {"type": "string"}, "path": {"type": "string"}, "side": {"type": "string", "enum": ["LEFT", "RIGHT"]}, "start_line": {"type": "integer"}, "end_line": {"type": "integer"}}}}}} - model: gpt-5.6-terra - effort: high + model: ${{ env.REVIEW_MODEL }} + effort: ${{ env.REVIEW_EFFORT }} # the CLI floated to npm latest too; pinned to the release verified # clean with v1.11 so a CLI release can't change review behaviour # under us either - codex-version: 0.149.0 + codex-version: ${{ env.CODEX_VERSION }} codex-home: .github/codex/home sandbox: danger-full-access # Allow reviews on PRs opened by github-actions[bot] (e.g. the @@ -317,7 +484,7 @@ jobs: id: estimate-cost env: CODEX_HOME: .github/codex/home - MODEL: gpt-5.6-terra + MODEL: ${{ env.REVIEW_MODEL }} # Standard text-token rates per 1M tokens, sourced from: # https://developers.openai.com/api/docs/models/gpt-5.6-terra PRICE_INPUT_PER_M: "2.0" @@ -327,7 +494,9 @@ jobs: set -euo pipefail echo "model=$MODEL" >> "$GITHUB_OUTPUT" - echo "price_source=https://developers.openai.com/api/docs/models/gpt-5.6-terra" >> "$GITHUB_OUTPUT" + if [ "$MODEL" = "gpt-5.6-terra" ]; then + echo "price_source=https://developers.openai.com/api/docs/models/gpt-5.6-terra" >> "$GITHUB_OUTPUT" + fi SESSION_FILE=$(find "$CODEX_HOME" -type f -name '*.jsonl' -print 2>/dev/null | xargs ls -t 2>/dev/null | head -n1 || true) if [ -z "$SESSION_FILE" ]; then @@ -357,12 +526,15 @@ jobs: OUTPUT_TOKENS=$(echo "$USAGE_JSON" | jq -r '.output_tokens // 0') TOTAL_TOKENS=$(echo "$USAGE_JSON" | jq -r '.total_tokens // (.input_tokens + .output_tokens)') - COST_USD=$(awk -v i="$INPUT_TOKENS" -v ci="$CACHED_INPUT_TOKENS" -v o="$OUTPUT_TOKENS" \ + COST_USD="" + if [ "$MODEL" = "gpt-5.6-terra" ]; then + COST_USD=$(awk -v i="$INPUT_TOKENS" -v ci="$CACHED_INPUT_TOKENS" -v o="$OUTPUT_TOKENS" \ -v pi="$PRICE_INPUT_PER_M" -v pci="$PRICE_CACHED_INPUT_PER_M" -v po="$PRICE_OUTPUT_PER_M" \ 'BEGIN { cost = ((i-ci)/1000000.0)*pi + (ci/1000000.0)*pci + (o/1000000.0)*po; printf "%.6f", cost }') + fi echo "usage_found=true" >> "$GITHUB_OUTPUT" echo "input_tokens=$INPUT_TOKENS" >> "$GITHUB_OUTPUT" @@ -374,11 +546,33 @@ jobs: echo "Token usage: input=$INPUT_TOKENS cached_input=$CACHED_INPUT_TOKENS output=$OUTPUT_TOKENS total=$TOTAL_TOKENS" echo "Estimated run cost (USD): $COST_USD" + - name: "๐Ÿ”ฌ Benchmark native review (artifact only)" + if: ${{ inputs.benchmark_native && steps.run-codex.outcome == 'success' }} + id: native-review + timeout-minutes: ${{ inputs.analysis_timeout_minutes || 12 }} + env: + CODEX_HOME: ${{ github.workspace }}/.github/codex/native-home + REVIEW_BASE: ${{ steps.select-range.outputs.compare_base }} + REVIEW_HEAD: ${{ steps.resolve-target.outputs.compare_head }} + REVIEW_MODE: ${{ steps.resolve-target.outputs.review_mode }} + run: | + set -euo pipefail + mkdir -p "$CODEX_HOME" + cp .github/codex/home/config.toml "$CODEX_HOME/config.toml" + git worktree add --detach "$RUNNER_TEMP/native-review" "$REVIEW_HEAD" + ARGS=(--base "$REVIEW_BASE") + [ "$REVIEW_MODE" = "commit" ] && ARGS=(--commit "$REVIEW_HEAD") + codex exec --cd "$RUNNER_TEMP/native-review" --model "$REVIEW_MODEL" \ + --config "model_reasoning_effort=\"$REVIEW_EFFORT\"" --sandbox read-only \ + --json --output-last-message "$GITHUB_WORKSPACE/.github/codex/output/native-review.md" \ + review "${ARGS[@]}" > .github/codex/output/native-events.jsonl + - name: "๐Ÿงพ Record analysis status" if: ${{ always() && steps.resolve-target.outputs.review_mode != 'skip' }} env: REVIEWED_SHA: ${{ steps.resolve-target.outputs.compare_head }} ANALYSIS_OUTCOME: ${{ steps.run-codex.outcome }} + NATIVE_OUTCOME: ${{ steps.native-review.outcome }} USAGE_FOUND: ${{ steps.estimate-cost.outputs.usage_found }} INPUT_TOKENS: ${{ steps.estimate-cost.outputs.input_tokens }} CACHED_INPUT_TOKENS: ${{ steps.estimate-cost.outputs.cached_input_tokens }} @@ -387,7 +581,8 @@ jobs: run: | mkdir -p .github/codex/output jq -n '{reviewed_sha:env.REVIEWED_SHA,analysis_outcome:env.ANALYSIS_OUTCOME, - model:"gpt-5.6-terra",effort:"high",codex_version:"0.149.0", + model:env.REVIEW_MODEL,effort:env.REVIEW_EFFORT,codex_version:env.CODEX_VERSION, + native_benchmark_outcome:env.NATIVE_OUTCOME, usage_found:env.USAGE_FOUND,input_tokens:env.INPUT_TOKENS, cached_input_tokens:env.CACHED_INPUT_TOKENS,output_tokens:env.OUTPUT_TOKENS, estimated_cost_usd:env.ESTIMATED_COST}' > .github/codex/output/analysis-status.json @@ -403,7 +598,10 @@ jobs: [ -n "${{ steps.resolve-target.outputs.skip_reason }}" ] && echo "- Skip reason: ${{ steps.resolve-target.outputs.skip_reason }}" echo "- Analysis outcome: ${{ steps.run-codex.outcome }}" echo "- Review output: review.json (validated and published in a separate job)" - echo "- Model: ${{ steps.estimate-cost.outputs.model || 'n/a' }}" + echo "- Model: $REVIEW_MODEL / $REVIEW_EFFORT / CLI $CODEX_VERSION" + echo "- Range: ${{ steps.select-range.outputs.range_kind }}" + echo "- Native benchmark: ${{ steps.native-review.outcome }}" + echo "- Dry run: ${{ inputs.dry_run || inputs.benchmark_native }}" if [ "${{ steps.estimate-cost.outputs.usage_found || 'false' }}" = "true" ]; then echo "- Token usage: input=${{ steps.estimate-cost.outputs.input_tokens }}, cached_input=${{ steps.estimate-cost.outputs.cached_input_tokens }}, output=${{ steps.estimate-cost.outputs.output_tokens }}, total=${{ steps.estimate-cost.outputs.total_tokens }}" echo "- Estimated model cost (USD): ${{ steps.estimate-cost.outputs.estimated_cost_usd }}" @@ -437,6 +635,12 @@ jobs: PR_NUMBER: ${{ needs.ai-review.outputs.pr_number }} REVIEW_HEAD: ${{ needs.ai-review.outputs.compare_head }} REVIEW_BASE: ${{ needs.ai-review.outputs.compare_base }} + FULL_BASE: ${{ needs.ai-review.outputs.full_base }} + REVIEW_PROFILE: ${{ needs.ai-review.outputs.review_profile }} + REVIEW_MODEL: ${{ inputs.review_model || 'gpt-5.6-terra' }} + REVIEW_EFFORT: ${{ inputs.review_effort || 'high' }} + CODEX_VERSION: ${{ inputs.codex_version || '0.149.0' }} + EXPECTED_BOT_LOGIN: ${{ inputs.review_bot_login || 'jitsu-code-review[bot]' }} REVIEW_REPO: ${{ github.workspace }}/reviewed-repo REVIEW_FILE: ${{ github.workspace }}/review-artifact/review.json STATUS_FILE: ${{ github.workspace }}/publication/status.json @@ -573,9 +777,12 @@ jobs: def publish(data, findings, repo, mode, subject, head): - digest = hashlib.sha256(json.dumps(data, sort_keys=True).encode()).hexdigest() + digest = hashlib.sha256(json.dumps([data, os.environ['REVIEW_PROFILE'], os.environ['REVIEW_BASE']], sort_keys=True).encode()).hexdigest() review_marker = f'' - summary = f"Review of `{head}`\n\n{data['summary']}\n\n{review_marker}" + coverage = json.dumps(dict(version=1, base=os.environ['FULL_BASE'], head=head, + profile=os.environ['REVIEW_PROFILE']), separators=(',', ':')) + summary = f"Review of `{head}`\n\n{data['summary']}" + completion = f"\n\n{review_marker}\n" if mode == 'pr': endpoint = f'repos/{repo}/pulls/{subject}' comments = api(endpoint + '/comments?per_page=100', paginate=True) @@ -583,26 +790,44 @@ jobs: login = os.environ['REVIEW_BOT_LOGIN'] own_comments = [c for c in comments if (c.get('user') or {}).get('login') == login] own_reviews = [r for r in reviews if (r.get('user') or {}).get('login') == login] - fresh = [f for f in findings if not any(marker(f) in c['body'] for c in own_comments)] + fresh = [f for f in findings if not any(marker(f) in (c.get('body') or '') for c in own_comments + own_reviews)] # Check after fetching history, immediately before any publication. current = api(endpoint) if current['state'] != 'open' or current['draft'] or current['head']['sha'] != head: raise ValueError('PR closed, draft, or changed head; stale review discarded') - if any(review_marker in (r.get('body') or '') for r in own_reviews) or (findings and not fresh): + if any(review_marker in (r.get('body') or '') for r in own_reviews): return 'duplicate', 0 - inline = [] + inline, parts = [], [summary] + full_ranges = changed_lines(os.environ['FULL_BASE'], head) for f in fresh: + # Incremental LEFT coordinates refer to the prior reviewed tree, + # not the PR base. Restored lines may not exist in the PR diff. + anchorable = (f['side'] == 'RIGHT' or os.environ['REVIEW_BASE'] == os.environ['FULL_BASE']) and any( + lo <= f['start_line'] <= f['end_line'] <= hi + for lo, hi in full_ranges.get(f['path'], {}).get(f['side'], [])) + if not anchorable: + block = '\n\n' + comment_body(f) + f"\n\n`{f['path']}:{f['start_line']}-{f['end_line']}` ({f['side']} of reviewed range)." + if len((parts[-1] + block + completion).encode()) > 60000: + parts.append(f"Additional findings for `{head}`") + parts[-1] += block + continue c = {'path': f['path'], 'side': f['side'], 'line': f['end_line'], 'body': comment_body(f)} if f['start_line'] != f['end_line']: c.update(start_line=f['start_line'], start_side=f['side']) inline.append(c) - result = api(endpoint + '/reviews', {'commit_id': head, 'event': 'APPROVE', 'body': summary, 'comments': inline}) - return 'published', len(inline) + # Only the final accepted part records complete coverage. On retry, + # fingerprints suppress findings from earlier accepted parts. + for index, body in enumerate(parts): + if index == len(parts) - 1: + body += completion + api(endpoint + '/reviews', {'commit_id': head, 'event': 'APPROVE', 'body': body, + 'comments': inline if index == 0 else []}) + return 'published', len(fresh) endpoint = f'repos/{repo}/commits/{head}/comments' comments = api(endpoint + '?per_page=100', paginate=True) if any(review_marker in c['body'] and c['user']['login'] == 'github-actions[bot]' for c in comments): return 'duplicate', 0 - body = summary + ''.join('\n\n' + comment_body(f) + f"\n\n`{f['path']}:{f['start_line']}-{f['end_line']}` ({f['side']})" for f in findings) + body = summary + completion + ''.join('\n\n' + comment_body(f) + f"\n\n`{f['path']}:{f['start_line']}-{f['end_line']}` ({f['side']})" for f in findings) if len(body) > 60000: raise ValueError('Commit review exceeds comment size limit') api(endpoint, {'body': body}) @@ -611,7 +836,9 @@ jobs: def main(): status = {'completion_status': 'failed', 'publication_status': 'not_posted', - 'reviewed_sha': os.environ['REVIEW_HEAD'], 'model': 'gpt-5.6-terra', 'effort': 'high'} + 'reviewed_sha': os.environ['REVIEW_HEAD'], 'compare_base': os.environ['REVIEW_BASE'], + 'model': os.environ['REVIEW_MODEL'], 'effort': os.environ['REVIEW_EFFORT'], + 'codex_version': os.environ['CODEX_VERSION'], 'review_profile': os.environ['REVIEW_PROFILE']} try: head, base = os.environ['REVIEW_HEAD'], os.environ['REVIEW_BASE'] if not re.fullmatch(r'[0-9a-f]{40}', head) or not re.fullmatch(r'[0-9a-f]{40}', base): @@ -649,7 +876,7 @@ jobs: # Mint posting credentials only after validation, on a runner that never ran Codex. - name: "๐Ÿ”‘ Generate GitHub App token for PR review" - if: ${{ env.REVIEW_MODE == 'pr' }} + if: ${{ env.REVIEW_MODE == 'pr' && !inputs.dry_run && !inputs.benchmark_native }} id: app-token uses: actions/create-github-app-token@v2 with: @@ -659,10 +886,16 @@ jobs: repositories: ${{ github.event.repository.name }} - name: "๐Ÿ’ฌ Publish validated review" + if: ${{ !inputs.dry_run && !inputs.benchmark_native }} env: GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }} REVIEW_BOT_LOGIN: ${{ steps.app-token.outputs.app-slug }}[bot] - run: python3 publication/publish.py + run: | + if [ "$REVIEW_MODE" = "pr" ] && [ "$REVIEW_BOT_LOGIN" != "$EXPECTED_BOT_LOGIN" ]; then + echo "Configured review_bot_login does not match the posting App" >&2 + exit 1 + fi + python3 publication/publish.py - name: "๐Ÿ“ Write publication summary" if: always() diff --git a/README.md b/README.md index 228aba0..d57121d 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,63 @@ name exported by analysis, including when only failed jobs are rerun: Only data is read from the analysis artifact; publisher code comes from the workflow. The model, reasoning effort, and CLI/action pins remain unchanged by this separation. +#### Incremental review and execution controls + +The publisher records an `ai-review-coverage` marker only after a validated review is +posted. An APPROVE verdict without that marker does not count as coverage. A follow-up +PR review starts at the last compatible reviewed SHA when it is an ancestor of the +current head and the full PR merge base is unchanged. First reviews, rebases, force +pushes, base changes, and incompatible settings fall back to the full PR diff. Same-head +reruns also review the full diff. Commit reviews remain parent-to-commit. + +Compatibility is a hash of the generated prompt, model, effort, CLI version, trusted bot +login, additional instructions, and PR title/body. Changes to these inputs require a +full review. Prior findings and dismissal explanations remain in the supplied context. +`range.json` in the context and artifact records both the selected range and full PR base. + +Incremental deleted-line coordinates refer to the prior reviewed tree. Such findings, +and restored lines absent from GitHub's overall PR diff, are included in the review +summary with their path/range instead of being posted at an incorrect inline location. +These summary findings do not create resolvable inline threads. Oversized summaries +are split into bounded reviews; only the final accepted part records coverage. Retries +suppress findings from earlier accepted parts. A new head still receives +a completion marker when all findings were already reported, without duplicate comments. + +These inputs are available through `workflow_call` and manual dispatch: + +| Input | Default | Purpose | +|-------|---------|---------| +| `review_model` | `gpt-5.6-terra` | Explicit model identifier | +| `review_effort` | `high` | `low`, `medium`, `high`, `xhigh`, or `max`; model must support it | +| `analysis_timeout_minutes` | `12` | Per-engine timeout; integer from 1 to 30 | +| `codex_version` | `0.149.0` | Pinned stable CLI version; floating versions are rejected | +| `incremental` | `true` | Set false to force a full comparison | +| `review_bot_login` | `jitsu-code-review[bot]` | Trusted completion-marker author; must match the posting App | +| `dry_run` | `false` | Validate/archive without posting or advancing coverage | +| `benchmark_native` | `false` | Add a native-review comparison; automatically suppresses publication | + +`benchmark_native` runs the structured prompt first, then `codex exec review` using a +clean worktree, the same selected range/model/effort/CLI, and a separate Codex home. The +native engine uses its built-in review guidance and repository AGENTS.md rather than the +custom prompt/PR discussion context. This compares the two execution setups; it does not +isolate prompt wording as the only variable. Both outputs are archived, including +`native-review.md` and `native-events.jsonl` (which includes native token usage events). +The structured result still passes the normal validator. Benchmark failure fails the run. + +Example: compare a newer pinned CLI without adopting it as the production default: + +```sh +gh workflow run ai-review.yml --repo jitsucom/github-workflows --ref \ + -f pr_number= -f codex_version=0.163.0 \ + -f incremental=false -f dry_run=true -f benchmark_native=true +``` + +For reusable callers, pass the same inputs under `with:`. Primary token usage is collected +before the native benchmark; other models retain usage counts but have no estimated +cost until their pricing is explicitly configured. No runtime or model default changes +are implied by a successful benchmark. Evaluate confirmed findings, false positives, +runtime and usage across representative PRs before adopting a new default. + #### Versioning Every merge to `main` is tagged `1..` by diff --git a/tests/test_review_context.py b/tests/test_review_context.py index c1c94d4..eea2809 100644 --- a/tests/test_review_context.py +++ b/tests/test_review_context.py @@ -115,8 +115,8 @@ def test_checkout_and_prompt_contract(self): self.assertIn('Intentional tradeoffs are context, not immunity', prompt) self.assertIn('Re-raise a dismissed finding only if new changes invalidate', prompt) self.assertNotIn('still prefer to post it', prompt) - self.assertIn('model: gpt-5.6-terra', text) - self.assertIn('codex-version: 0.149.0', text) + self.assertIn("REVIEW_MODEL: ${{ inputs.review_model || 'gpt-5.6-terra' }}", text) + self.assertIn("CODEX_VERSION: ${{ inputs.codex_version || '0.149.0' }}", text) if __name__ == '__main__': diff --git a/tests/test_review_publisher.py b/tests/test_review_publisher.py index 7e36401..33864d3 100644 --- a/tests/test_review_publisher.py +++ b/tests/test_review_publisher.py @@ -43,7 +43,8 @@ def setUp(self): REVIEW_BASE=self.base, REPO='owner/repo', REVIEW_MODE='pr', PR_NUMBER='7', REVIEW_FILE=str(self.path / 'review.json'), STATUS_FILE=str(self.path / 'publication/status.json'), - REVIEW_BOT_LOGIN='review-bot[bot]') + REVIEW_BOT_LOGIN='review-bot[bot]', FULL_BASE=self.base, REVIEW_PROFILE='test-profile', + REVIEW_MODEL='gpt-5.6-terra', REVIEW_EFFORT='high', CODEX_VERSION='0.149.0') self.env.start() self.addCleanup(self.env.stop) self.data = {'version': 1, 'reviewed_sha': self.head, 'completion_status': 'complete', @@ -114,6 +115,46 @@ def test_quoted_paths_and_header_like_source_text(self): self.data.update(reviewed_sha=head, findings=[dict(self.f, path=path.name, start_line=1, end_line=1)]) self.assertEqual(len(self.p.validate(self.data, head, base)), 1) + def test_incremental_restoration_uses_summary_and_records_coverage(self): + original_base, previous = self.base, self.head + (self.repo / 'file with spaces.txt').write_text('keep\nold\nlast\n') + self.git('add', '.') + self.git('commit', '-m', 'restore original lines') + self.head = self.git('rev-parse', 'HEAD') + self.base = previous + self.data.update(reviewed_sha=self.head, findings=[dict(self.f, side='LEFT', start_line=2, end_line=2)]) + with patch.dict(os.environ, REVIEW_HEAD=self.head, REVIEW_BASE=previous, FULL_BASE=original_base), patch.object(self.p, 'api', self.api): + self.live['head']['sha'] = self.head + self.assertEqual(self.publish(), ('published', 1)) + payload = self.posts[-1][1] + self.assertEqual(payload['comments'], []) + self.assertIn(self.f['body'], payload['body']) + self.assertIn('LEFT of reviewed range', payload['body']) + self.assertIn('ai-review-coverage:', payload['body']) + self.assertIn('"base":"' + original_base + '"', payload['body']) + + def test_large_incremental_summary_parts_record_coverage_only_at_end(self): + self.findings() + # Force legitimate delta findings into summary mode; validation is exercised + # separately by the restoration fixture above. + findings = [dict(self.p.validate(self.data, self.head, self.base)[0], + title='Finding ' + str(i), body='x' * 6000, fingerprint=str(i)) for i in range(12)] + with patch.dict(os.environ, REVIEW_BASE=self.head), patch.object(self.p, 'changed_lines', return_value={}), patch.object(self.p, 'api', self.api): + self.assertEqual(self.p.publish(self.data, findings, 'owner/repo', 'pr', '7', self.head), ('published', 12)) + self.assertGreater(len(self.posts), 1) + for _, payload in self.posts: + self.assertLessEqual(len(payload['body'].encode()), 60000) + self.assertNotIn('ai-review-coverage:', self.posts[0][1]['body']) + self.assertIn('ai-review-coverage:', self.posts[-1][1]['body']) + # Simulate the first part succeeding and the final POST failing. + prior = self.posts[0][1]['body'] + self.posts.clear() + self.reviews = [{'user': {'login': 'review-bot[bot]'}, 'body': prior}] + with patch.dict(os.environ, REVIEW_BASE=self.head), patch.object(self.p, 'changed_lines', return_value={}), patch.object(self.p, 'api', self.api): + self.p.publish(self.data, findings, 'owner/repo', 'pr', '7', self.head) + self.assertNotIn('ai-review-finding:0 -->', self.posts[0][1]['body']) + self.assertIn('ai-review-coverage:', self.posts[-1][1]['body']) + def test_invalid_output_is_never_published(self): cases = [None, {}, dict(self.data, completion_status='incomplete'), dict(self.data, reviewed_sha=self.base), dict(self.data, version=True)] @@ -164,8 +205,8 @@ def test_duplicates_with_shifted_lines_and_other_authors(self): self.assertEqual(len(self.p.validate(self.data, self.head, self.base)), 1) self.history = [{'user': {'login': 'review-bot[bot]'}, 'body': self.p.comment_body(f), 'line': 999}] with patch.object(self.p, 'api', self.api): - self.assertEqual(self.publish(), ('duplicate', 0)) - self.assertFalse(self.posts) + self.assertEqual(self.publish(), ('published', 0)) + self.assertEqual(self.posts[-1][1]['comments'], []) self.history[0]['user']['login'] = 'another-user' with patch.object(self.p, 'api', self.api): self.assertEqual(self.publish(), ('published', 1)) diff --git a/tests/test_review_range.py b/tests/test_review_range.py new file mode 100644 index 0000000..b8f20b8 --- /dev/null +++ b/tests/test_review_range.py @@ -0,0 +1,196 @@ +"""Regression checks for coverage selection and artifact-only execution.""" +import importlib.util +import json +import os +from pathlib import Path +import subprocess +import tempfile +import unittest +from unittest.mock import patch + +from test_review_context import run_block, WORKFLOW + + +def load_range(root): + script = run_block('๐Ÿ“ Select review range').split("<<'PYTHON_EOF'\n", 1)[1].rsplit('\nPYTHON_EOF', 1)[0] + path = root / 'select_range.py' + path.write_text(script) + spec = importlib.util.spec_from_file_location('select_range', path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class ReviewRangeTest(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.module = load_range(self.root) + self.git('init', '-b', 'main') + self.git('config', 'user.name', 'Test') + self.git('config', 'user.email', 'test@example.com') + self.base = self.commit('base.txt') + self.previous = self.commit('previous.txt') + self.head = self.commit('fixup.txt') + self.bot = 'review-bot[bot]' + self.reviews = [] + self.env = dict(REVIEW_MODE='pr', COMPARE_HEAD=self.head, FULL_BASE=self.base, + REVIEW_MODEL='gpt-5.6-terra', REVIEW_EFFORT='high', CODEX_VERSION='0.149.0', + REVIEW_BOT_LOGIN=self.bot, INCREMENTAL='true', REPO='owner/repo', + PR_NUMBER='7', GITHUB_OUTPUT=str(self.root / 'outputs')) + self.ctx = self.root / '.github/codex/context' + self.ctx.mkdir(parents=True) + (self.root / '.github/codex/output').mkdir() + (self.root / '.github/workflows').mkdir() + (self.root / '.github/workflows/ai-review.prompt.md').write_text('Trusted review prompt') + (self.ctx / 'pr.json').write_text(json.dumps(dict(title='Fix', body='Intent', headRefOid=self.head))) + (self.ctx / 'additional-instructions.txt').write_text('Focus') + (self.ctx / 'review-target.txt').write_text('compare_base: ' + self.base + '\n') + self.original_check_output = subprocess.check_output + + def git(self, *args): + return subprocess.check_output(['git', *args], cwd=self.root, text=True, stderr=subprocess.DEVNULL).strip() + + def commit(self, name): + (self.root / name).write_text(name) + self.git('add', name) + self.git('commit', '-m', name) + return self.git('rev-parse', 'HEAD') + + def gh(self, args, **kwargs): + if args[0] == 'gh': + self.assertIn('--paginate', args) + return json.dumps([self.reviews[:1], self.reviews[1:]]) + return self.original_check_output(args, **kwargs) + + def select(self): + old = os.getcwd() + try: + os.chdir(self.root) + with patch.dict(os.environ, self.env), patch.object(self.module.subprocess, 'check_output', self.gh): + self.module.main() + finally: + os.chdir(old) + return json.loads((self.ctx / 'range.json').read_text()) + + def coverage(self, info, **changes): + values = dict(version=1, base=self.base, head=self.previous, profile=info['review_profile']) + values.update(changes) + return dict(user={'login': self.bot}, state='APPROVED', commit_id=values['head'], + body='') + + def test_first_review_full_then_fixup_incremental(self): + info = self.select() + self.assertEqual(info['range_kind'], 'full') + self.reviews = [self.coverage(info)] + info = self.select() + self.assertEqual(info['compare_base'], self.previous) + self.assertEqual(info['range_kind'], 'incremental') + self.assertEqual((self.root / '.github/codex/output/diff-files.txt').read_text(), 'fixup.txt\n') + self.assertEqual(json.loads((self.root / '.github/codex/output/range.json').read_text()), info) + + def test_force_push_and_changed_base_fall_back_to_full(self): + info = self.select() + self.git('checkout', '-b', 'rewritten', self.base) + rewritten = self.commit('rewritten.txt') + self.reviews = [self.coverage(info, head=rewritten)] + self.assertEqual(self.select()['range_kind'], 'full') + self.reviews = [self.coverage(info, base=rewritten)] + self.assertEqual(self.select()['range_kind'], 'full') + + def test_settings_prompt_and_intent_changes_require_full_review(self): + info = self.select() + self.reviews = [self.coverage(info)] + for key, value in [('REVIEW_MODEL', 'gpt-6-astra'), ('REVIEW_EFFORT', 'medium'), ('CODEX_VERSION', '0.159.1')]: + with self.subTest(key=key): + old = self.env[key] + self.env[key] = value + self.assertEqual(self.select()['range_kind'], 'full') + self.env[key] = old + (self.ctx / 'pr.json').write_text(json.dumps(dict(title='Fix', body='Changed intent', headRefOid=self.head))) + self.assertEqual(self.select()['range_kind'], 'full') + + def test_approval_without_trusted_matching_coverage_is_not_progress(self): + info = self.select() + cases = [dict(user={'login': self.bot}, state='APPROVED', commit_id=self.previous, body='Looks good'), + dict(self.coverage(info), user={'login': 'someone-else'}), + dict(self.coverage(info), commit_id=self.head), + dict(self.coverage(info), state='PENDING')] + for review in cases: + with self.subTest(review=review): + self.reviews = [review] + self.assertEqual(self.select()['range_kind'], 'full') + + def test_explicit_full_and_commit_mode_ignore_coverage(self): + info = self.select() + self.reviews = [self.coverage(info)] + self.env['INCREMENTAL'] = 'false' + self.assertEqual(self.select()['range_kind'], 'full') + self.env.update(INCREMENTAL='true', REVIEW_MODE='commit') + self.assertEqual(self.select()['range_kind'], 'full') + + def test_history_api_failure_does_not_silently_advance(self): + old = os.getcwd() + try: + os.chdir(self.root) + with patch.dict(os.environ, self.env), patch.object(self.module.subprocess, 'check_output', side_effect=subprocess.CalledProcessError(1, ['gh'])), self.assertRaises(subprocess.CalledProcessError): + self.module.main() + finally: + os.chdir(old) + + def test_native_benchmark_command_and_clean_tree(self): + home = self.root / '.github/codex/home' + home.mkdir() + (home / 'config.toml').write_text('[model_providers.test]\nbase_url="http://localhost:1234"\n') + bin_dir = self.root / 'bin' + bin_dir.mkdir() + (bin_dir / 'codex').write_text('#!/bin/sh\nprintf "%s\\n" "$@" > "$GITHUB_WORKSPACE/native-args.txt"\nprintf \'{"type":"turn.completed"}\\n\'\n') + (bin_dir / 'codex').chmod(0o755) + env = dict(os.environ, PATH=str(bin_dir) + os.pathsep + os.environ['PATH'], + CODEX_HOME=str(self.root / 'native-home'), RUNNER_TEMP=str(self.root / 'runner'), + GITHUB_WORKSPACE=str(self.root), REVIEW_MODE='pr', REVIEW_BASE=self.base, + REVIEW_HEAD=self.head, REVIEW_MODEL='gpt-5.6-terra', REVIEW_EFFORT='high') + subprocess.run(['bash', '-e', '-c', run_block('๐Ÿ”ฌ Benchmark native review (artifact only)')], + cwd=self.root, env=env, capture_output=True, check=True) + args = (self.root / 'native-args.txt').read_text().splitlines() + self.assertEqual(args[-3:], ['review', '--base', self.base]) + self.assertIn('model_reasoning_effort="high"', args) + self.assertEqual(self.git('-C', str(self.root / 'runner/native-review'), 'rev-parse', 'HEAD'), self.head) + self.assertTrue((self.root / '.github/codex/output/native-events.jsonl').exists()) + + def test_cost_estimate_is_not_reused_for_other_models(self): + home = self.root / 'sessions' + home.mkdir() + (home / 'session.jsonl').write_text(json.dumps({'type':'event_msg', 'payload':{'type':'token_count', 'info':{'total_token_usage':{'input_tokens':1000000,'cached_input_tokens':500000,'output_tokens':100000}}}}) + '\n') + for model in ('gpt-5.6-terra', 'gpt-6-astra'): + output = self.root / ('cost-' + model) + env = dict(os.environ, CODEX_HOME=str(home), MODEL=model, GITHUB_OUTPUT=str(output), + PRICE_INPUT_PER_M='2.0', PRICE_CACHED_INPUT_PER_M='0.2', PRICE_OUTPUT_PER_M='12.0') + subprocess.run(['bash', '-e', '-c', run_block('๐Ÿ’ฐ Estimate token usage and cost')], + cwd=self.root, env=env, check=True, capture_output=True) + data = dict(line.split('=', 1) for line in output.read_text().splitlines()) + self.assertEqual(data['input_tokens'], '1000000') + self.assertEqual(data['estimated_cost_usd'], '2.300000' if model == 'gpt-5.6-terra' else '') + + def test_execution_settings_are_pinned_and_bounded(self): + for changes, valid in [({}, True), ({'CODEX_VERSION':'latest'}, False), + ({'TIMEOUT_MINUTES':'0'}, False), ({'TIMEOUT_MINUTES':'31'}, False), + ({'REVIEW_EFFORT':'unknown'}, False), ({'CODEX_VERSION':'0.163.0'}, True)]: + env = dict(os.environ, REVIEW_MODEL='gpt-5.6-terra', REVIEW_EFFORT='high', + CODEX_VERSION='0.149.0', TIMEOUT_MINUTES='12') + env.update(changes) + result = subprocess.run(['bash', '-e', '-c', run_block('โš™๏ธ Validate execution settings')], + cwd=self.root, env=env, capture_output=True) + self.assertEqual(result.returncode == 0, valid, changes) + + def test_dry_run_gates_credentials_and_publication(self): + text = WORKFLOW.read_text() + self.assertIn("INCREMENTAL: ${{ toJSON(inputs.incremental) != 'false' }}", text) + self.assertIn("env.REVIEW_MODE == 'pr' && !inputs.dry_run && !inputs.benchmark_native", text) + self.assertIn("if: ${{ !inputs.dry_run && !inputs.benchmark_native }}", text) + self.assertIn('output-schema:', text) + + +if __name__ == '__main__': + unittest.main()