From 831518027107f6032e6b6cfae564edf02cdaa833 Mon Sep 17 00:00:00 2001 From: w0nk1 Date: Mon, 16 Feb 2026 14:58:15 +0100 Subject: [PATCH] feat: automate codex feedback loop and localize eligibility --- .cursor/commands/pr-review.md | 56 ++++++++ .github/workflows/codex-feedback-loop.yml | 142 +++++++++++++++++++ CONTRIBUTING.md | 6 + docs/breadcrumbs.md | 6 + docs/choices.md | 5 + docs/specs/2026-02-16-codex-feedback-loop.md | 19 +++ src-tauri/src/i18n.rs | 79 +++++++++++ src-tauri/src/lib.rs | 52 +++---- src/components/SettingsSheet.test.tsx | 8 ++ src/components/SettingsSheet.tsx | 9 +- 10 files changed, 353 insertions(+), 29 deletions(-) create mode 100644 .cursor/commands/pr-review.md create mode 100644 .github/workflows/codex-feedback-loop.yml create mode 100644 docs/specs/2026-02-16-codex-feedback-loop.md diff --git a/.cursor/commands/pr-review.md b/.cursor/commands/pr-review.md new file mode 100644 index 0000000..f7f9da3 --- /dev/null +++ b/.cursor/commands/pr-review.md @@ -0,0 +1,56 @@ +--- +description: Review a PR by number - checkout, compare against main, run security checks, and return merge guidance +args: + - name: pr + description: PR number (e.g. 42) + required: true +--- + +Review PR #{{pr}} with this workflow: + +## 1. Gather context + +Run in parallel: +- `gh pr view {{pr}} --json title,body,headRefName,baseRefName,files,additions,deletions,author` +- Read relevant baseline files in current `main` (affected frontend components, tauri commands, workflows, tests) +- Read translation/i18n and release policy docs if touched (`CONTRIBUTING.md`, `docs/choices.md`, `docs/breadcrumbs.md`) + +## 2. Checkout and read complete diff + +- `gh pr checkout {{pr}}` +- Read all changed files in the PR, not only highlighted hunks +- Validate tests and workflows impacted by the change + +## 3. Review checklist + +Check: +- Correctness and regression risk (frontend + tauri + swift bridge where applicable) +- Security and privacy impact (permissions, commands, external calls, secrets) +- i18n consistency (no mixed-language output for selected app language) +- CI compatibility (coverage thresholds, clippy, build) +- Contributor ergonomics (docs/scripts updated when contributor workflow changes) +- Test coverage for changed behavior (new branches and failure paths) + +## 4. Classify findings + +Group by severity: +- Blocker: security/data loss/broken release behavior +- High: likely user-visible bug or broken contributor flow +- Medium: convention mismatch, missing test, maintainability risk +- Low: minor cleanup/nits + +## 5. Present result and action options + +Return: +- Findings first, sorted by severity, with file references +- Explicit recommendation: + 1. Request changes + 2. Comment-only follow-up + 3. Safe to merge + +If user chooses review posting: +- Use `gh pr review {{pr}} --comment` (or `--request-changes` if needed) +- Keep comment concise and actionable + +After review actions: +- `git checkout main` diff --git a/.github/workflows/codex-feedback-loop.yml b/.github/workflows/codex-feedback-loop.yml new file mode 100644 index 0000000..27e82a7 --- /dev/null +++ b/.github/workflows/codex-feedback-loop.yml @@ -0,0 +1,142 @@ +name: Codex Feedback Loop + +on: + pull_request_review: + types: [submitted] + issue_comment: + types: [created] + workflow_dispatch: + inputs: + pr_number: + description: "PR number to process (required for manual runs)" + required: true + type: string + review_id: + description: "Optional review id (limits findings to one review)" + required: false + type: string + +permissions: + contents: read + issues: write + pull-requests: write + +jobs: + request-codex-fix: + if: >- + github.event_name == 'workflow_dispatch' || + (github.event_name == 'pull_request_review' && + github.event.review.user.login == 'chatgpt-codex-connector[bot]' && + github.event.review.state == 'commented') || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + startsWith(github.event.comment.body, '/codex-fix')) + runs-on: ubuntu-latest + steps: + - name: Resolve PR and review context + id: ctx + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + PR="${{ inputs.pr_number }}" + REVIEW_ID="${{ inputs.review_id }}" + elif [ "${{ github.event_name }}" = "pull_request_review" ]; then + PR="${{ github.event.pull_request.number }}" + REVIEW_ID="${{ github.event.review.id }}" + else + PR="${{ github.event.issue.number }}" + REVIEW_ID="" + fi + + if [ -z "$PR" ]; then + echo "Missing PR number." + exit 1 + fi + + STATE=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json state --jq .state) + echo "pr_number=$PR" >> "$GITHUB_OUTPUT" + echo "review_id=$REVIEW_ID" >> "$GITHUB_OUTPUT" + echo "pr_state=$STATE" >> "$GITHUB_OUTPUT" + + - name: Stop if PR is not open + if: steps.ctx.outputs.pr_state != 'OPEN' + run: echo "PR #${{ steps.ctx.outputs.pr_number }} is not open; skipping." + + - name: Collect codex findings + if: steps.ctx.outputs.pr_state == 'OPEN' + id: findings + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ steps.ctx.outputs.pr_number }} + REVIEW_ID: ${{ steps.ctx.outputs.review_id }} + run: | + if [ -n "$REVIEW_ID" ]; then + RAW=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews/$REVIEW_ID/comments" --paginate) + FINDINGS=$(echo "$RAW" | jq '[.[] | select(.user.login=="chatgpt-codex-connector[bot]")]') + else + RAW=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/comments" --paginate) + FINDINGS=$(echo "$RAW" | jq '[.[] | select(.user.login=="chatgpt-codex-connector[bot]")]') + fi + + COUNT=$(echo "$FINDINGS" | jq 'length') + echo "count=$COUNT" >> "$GITHUB_OUTPUT" + + if [ "$COUNT" -eq 0 ]; then + echo "No codex review findings." + exit 0 + fi + + SUMMARY=$(echo "$FINDINGS" | jq -r ' + .[:5] + | map("- `" + (.path // "unknown") + ":" + ((.line // 0) | tostring) + "` " + (.body | gsub("\\n"; " ") | .[0:180])) + | join("\n") + ') + + { + echo "summary<> "$GITHUB_OUTPUT" + + - name: Prevent duplicate autofix request + if: steps.ctx.outputs.pr_state == 'OPEN' && steps.findings.outputs.count != '0' + id: dedupe + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ steps.ctx.outputs.pr_number }} + REVIEW_ID: ${{ steps.ctx.outputs.review_id }} + run: | + MARKER="" + FOUND=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" --paginate --jq ".[] | select(.body | contains(\"$MARKER\")) | .id" | head -n 1 || true) + echo "marker=$MARKER" >> "$GITHUB_OUTPUT" + if [ -n "$FOUND" ]; then + echo "already=true" >> "$GITHUB_OUTPUT" + else + echo "already=false" >> "$GITHUB_OUTPUT" + fi + + - name: Ask codex to apply feedback + if: steps.ctx.outputs.pr_state == 'OPEN' && steps.findings.outputs.count != '0' && steps.dedupe.outputs.already != 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ steps.ctx.outputs.pr_number }} + MARKER: ${{ steps.dedupe.outputs.marker }} + SUMMARY: ${{ steps.findings.outputs.summary }} + COUNT: ${{ steps.findings.outputs.count }} + run: | + gh pr comment "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --body "$(cat <`) instead of segmented pills; reason: scales for many locales and avoids crowded/overflowing controls as translators add languages. - Default: language options are sourced from dynamically discovered locale catalogs (`availableLocales`) plus `system`; reason: contributor flow should be "add locale file only" with no UI code edits. - Default: cross-webview `language-changed` handling validates locale via `isSupportedAppLanguage` instead of hardcoded `en/de`; reason: runtime should automatically accept newly added locale files. + +2026-02-16 (codex-review-loop-and-eligibility-locale-fix) +- Default: codex feedback automation is follow-up-by-comment (`@codex address that feedback`) with dedupe markers, not direct auto-commit from workflow; reason: keeps human oversight and avoids unsafe write automation in CI context. +- Default: codex follow-up workflow supports three entry points (codex review submit, `/codex-fix` comment, manual dispatch); reason: reliable trigger paths for both automatic and operator-driven remediation. +- Default: Apple Intelligence eligibility uses selected app language (resolved `en|de`) instead of backend `system_locale`; reason: prevents mixed-language eligibility reasons in localized Settings UI. diff --git a/docs/specs/2026-02-16-codex-feedback-loop.md b/docs/specs/2026-02-16-codex-feedback-loop.md new file mode 100644 index 0000000..62bbb01 --- /dev/null +++ b/docs/specs/2026-02-16-codex-feedback-loop.md @@ -0,0 +1,19 @@ +# Codex Feedback Loop + Review Playbook + +## Scope +- Add a reusable PR review playbook command (`.cursor/commands/pr-review.md`). +- Add automated codex follow-up workflow for review findings. +- Fix mixed-language AI eligibility reasons by honoring selected app language. + +## Design +- Workflow `.github/workflows/codex-feedback-loop.yml`: + - Triggers on codex review submission, `/codex-fix` PR comments, and manual dispatch. + - Collects codex inline findings from PR review comments. + - Posts a deduplicated follow-up comment: `@codex address that feedback`. +- Settings frontend passes resolved app language into `get_apple_intelligence_eligibility`. +- Rust backend resolves that language and localizes eligibility reason strings. + +## Verification +- `npm test` +- `cd src-tauri && cargo test --quiet` +- Workflow syntax sanity via local YAML inspection diff --git a/src-tauri/src/i18n.rs b/src-tauri/src/i18n.rs index fd84dbc..ce303da 100644 --- a/src-tauri/src/i18n.rs +++ b/src-tauri/src/i18n.rs @@ -164,6 +164,69 @@ pub fn auth_placeholder_description(locale: Locale) -> &'static str { } } +pub fn ai_eligibility_requires_apple_silicon(locale: Locale) -> &'static str { + match locale { + Locale::En => "Requires Apple Silicon (M1+).", + Locale::De => "Erfordert Apple Silicon (M1+).", + } +} + +pub fn ai_eligibility_unknown_macos_version(locale: Locale) -> &'static str { + match locale { + Locale::En => "Could not detect macOS version.", + Locale::De => "macOS-Version konnte nicht erkannt werden.", + } +} + +pub fn ai_eligibility_requires_macos_26(locale: Locale) -> &'static str { + match locale { + Locale::En => "Requires macOS 26+.", + Locale::De => "Erfordert macOS 26+.", + } +} + +pub fn ai_eligibility_check_failed(locale: Locale) -> &'static str { + match locale { + Locale::En => "Could not check Apple Intelligence availability.", + Locale::De => "Apple-Intelligence-Verfugbarkeit konnte nicht gepruft werden.", + } +} + +pub fn ai_eligibility_available(locale: Locale) -> &'static str { + match locale { + Locale::En => "Available.", + Locale::De => "Verfugbar.", + } +} + +pub fn ai_eligibility_not_enabled(locale: Locale) -> &'static str { + match locale { + Locale::En => "Apple Intelligence is disabled in System Settings.", + Locale::De => "Apple Intelligence ist in den Systemeinstellungen deaktiviert.", + } +} + +pub fn ai_eligibility_device_not_eligible(locale: Locale) -> &'static str { + match locale { + Locale::En => "This device is not eligible for Apple Intelligence.", + Locale::De => "Dieses Gerat ist nicht fur Apple Intelligence geeignet.", + } +} + +pub fn ai_eligibility_model_not_ready(locale: Locale) -> &'static str { + match locale { + Locale::En => "Apple Intelligence model is not ready yet (downloading/initializing).", + Locale::De => "Apple-Intelligence-Modell ist noch nicht bereit (Download/Initialisierung).", + } +} + +pub fn ai_eligibility_unavailable(locale: Locale) -> &'static str { + match locale { + Locale::En => "Apple Intelligence unavailable.", + Locale::De => "Apple Intelligence ist nicht verfugbar.", + } +} + pub fn tray_tooltip(_locale: Locale) -> &'static str { "StepCast" } @@ -251,4 +314,20 @@ mod tests { "StepCast - Aufnahme läuft..." ); } + + #[test] + fn ai_eligibility_helpers_render_translated_strings() { + assert_eq!( + ai_eligibility_requires_apple_silicon(Locale::En), + "Requires Apple Silicon (M1+)." + ); + assert_eq!( + ai_eligibility_requires_apple_silicon(Locale::De), + "Erfordert Apple Silicon (M1+)." + ); + assert_eq!( + ai_eligibility_not_enabled(Locale::De), + "Apple Intelligence ist in den Systemeinstellungen deaktiviert." + ); + } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 3b584aa..150a955 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -122,7 +122,9 @@ fn macos_product_version() -> Option { } #[tauri::command] -fn get_apple_intelligence_eligibility() -> AppleIntelligenceEligibility { +fn get_apple_intelligence_eligibility( + app_language: Option, +) -> AppleIntelligenceEligibility { let arch = std::env::consts::ARCH; #[cfg(not(target_os = "macos"))] @@ -136,6 +138,7 @@ fn get_apple_intelligence_eligibility() -> AppleIntelligenceEligibility { #[cfg(target_os = "macos")] { + let locale = i18n::resolve_locale(i18n::parse_app_language(app_language.as_deref())); let version = macos_product_version(); let platform_details = version .as_ref() @@ -145,7 +148,7 @@ fn get_apple_intelligence_eligibility() -> AppleIntelligenceEligibility { if arch != "aarch64" { return AppleIntelligenceEligibility { eligible: false, - reason: "Requires Apple Silicon (M1+).".to_string(), + reason: i18n::ai_eligibility_requires_apple_silicon(locale).to_string(), details: platform_details, }; } @@ -157,7 +160,7 @@ fn get_apple_intelligence_eligibility() -> AppleIntelligenceEligibility { if major.is_none() { return AppleIntelligenceEligibility { eligible: false, - reason: "Could not detect macOS version.".to_string(), + reason: i18n::ai_eligibility_unknown_macos_version(locale).to_string(), details: platform_details, }; } @@ -165,49 +168,46 @@ fn get_apple_intelligence_eligibility() -> AppleIntelligenceEligibility { if major.unwrap_or(0) < 26 { return AppleIntelligenceEligibility { eligible: false, - reason: "Requires macOS 26+.".to_string(), + reason: i18n::ai_eligibility_requires_macos_26(locale).to_string(), details: platform_details, }; } - let availability = - match crate::apple_intelligence::availability(Some(crate::i18n::system_locale())) { - Ok(a) => a, - Err(err) => { - return AppleIntelligenceEligibility { - eligible: false, - reason: "Could not check Apple Intelligence availability.".to_string(), - details: Some(format!( - "{}; {}", - platform_details.unwrap_or_else(|| format!("macos (unknown) ({arch})")), - err - )), - }; - } - }; + let availability = match crate::apple_intelligence::availability(Some(locale)) { + Ok(a) => a, + Err(err) => { + return AppleIntelligenceEligibility { + eligible: false, + reason: i18n::ai_eligibility_check_failed(locale).to_string(), + details: Some(format!( + "{}; {}", + platform_details.unwrap_or_else(|| format!("macos (unknown) ({arch})")), + err + )), + }; + } + }; if availability.available { return AppleIntelligenceEligibility { eligible: true, - reason: "Available.".to_string(), + reason: i18n::ai_eligibility_available(locale).to_string(), details: platform_details, }; } let reason = match availability.reason.as_deref() { Some("appleIntelligenceNotEnabled") => { - "Apple Intelligence is disabled in System Settings.".to_string() + i18n::ai_eligibility_not_enabled(locale).to_string() } Some("deviceNotEligible") => { - "This device is not eligible for Apple Intelligence.".to_string() - } - Some("modelNotReady") => { - "Apple Intelligence model is not ready yet (downloading/initializing).".to_string() + i18n::ai_eligibility_device_not_eligible(locale).to_string() } + Some("modelNotReady") => i18n::ai_eligibility_model_not_ready(locale).to_string(), _ => availability .details .clone() - .unwrap_or_else(|| "Apple Intelligence unavailable.".to_string()), + .unwrap_or_else(|| i18n::ai_eligibility_unavailable(locale).to_string()), }; let mut details = platform_details; diff --git a/src/components/SettingsSheet.test.tsx b/src/components/SettingsSheet.test.tsx index a57c5cf..be849bc 100644 --- a/src/components/SettingsSheet.test.tsx +++ b/src/components/SettingsSheet.test.tsx @@ -77,6 +77,14 @@ describe("SettingsSheet", () => { expect(mockEmit).toHaveBeenCalledWith("language-changed", { language: "de" }); }); + it("passes selected app language to Apple Intelligence eligibility check", async () => { + localStorage.setItem("appLanguage", "de"); + renderSettings(); + expect(mockInvoke).toHaveBeenCalledWith("get_apple_intelligence_eligibility", { + appLanguage: "de", + }); + }); + it("sets data-theme and localStorage when selecting a theme", async () => { const user = userEvent.setup(); renderSettings(); diff --git a/src/components/SettingsSheet.tsx b/src/components/SettingsSheet.tsx index 76be344..dc6908b 100644 --- a/src/components/SettingsSheet.tsx +++ b/src/components/SettingsSheet.tsx @@ -63,7 +63,7 @@ export function initTheme() { } export default function SettingsSheet({ onBack }: SettingsSheetProps) { - const { appLanguage, availableLocales, setAppLanguage, getLanguageLabel, t } = useI18n(); + const { appLanguage, locale, availableLocales, setAppLanguage, getLanguageLabel, t } = useI18n(); const [theme, setTheme] = useState( () => (localStorage.getItem("theme") as Theme) || "system" ); @@ -89,7 +89,10 @@ export default function SettingsSheet({ onBack }: SettingsSheetProps) { }, [appLanguage]); useEffect(() => { - invoke("get_apple_intelligence_eligibility") + const eligibilityLanguage = appLanguage === "system" ? locale : appLanguage; + invoke("get_apple_intelligence_eligibility", { + appLanguage: eligibilityLanguage, + }) .then((result) => { if (result && typeof result.eligible === "boolean" && typeof result.reason === "string") { setAiEligibility(result); @@ -98,7 +101,7 @@ export default function SettingsSheet({ onBack }: SettingsSheetProps) { } }) .catch(() => setAiEligibility({ eligible: false, reason: t("settings.ai.eligibility.fallback") })); - }, [t]); + }, [appLanguage, locale, t]); const selectTheme = useCallback((t: Theme) => { setTheme(t);