Skip to content

[1905 by GPT-5.6 Sol] Make /use change only the PR model label - #1909

Open
propr-dev[bot] wants to merge 8 commits into
mainfrom
1905/gpt-5.6-sol-make-use-change-only-the--20260814-2046-swx
Open

[1905 by GPT-5.6 Sol] Make /use change only the PR model label#1909
propr-dev[bot] wants to merge 8 commits into
mainfrom
1905/gpt-5.6-sol-make-use-change-only-the--20260814-2046-swx

Conversation

@propr-dev

@propr-dev propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

AI Implementation Summary

Closes #1905

Branch: 1905/gpt-5.6-sol-make-use-change-only-the--20260814-2046-swx
Commits: ✅ Changes committed (540bacc)


AI Processing Completed

Execution Details:

  • Issue: Make /use change only the PR model label #1905
  • Repository: integry/propr
  • Status: Success
  • Execution Time: 20m 57s
  • Tokens used: 13.49M tokens [13.46M input + 25.5K output]
  • API cost: $8.72
  • Timestamp: Aug 14, 2026, 9:07 PM UTC
  • LLM Model: GPT-5.6 Sol

Summary:
Implemented issue #1905.

Key changes:

  • /use <model-or-label> now updates only the PR’s canonical managed model label.
  • Resolves short aliases and full labels against enabled agents.
  • Preserves all unrelated labels.
  • Rejects missing, unknown, disabled, or unsupported targets.
  • Handles same-label selection as an idempotent no-op.
  • Never enqueues, batches, cancels, or persists command/model context—even with trailing text.
  • Leaves /switch behavior unchanged.
  • Updated command help and documentation to describe the new behavior and default-model routing.

Validation:

  • Focused parser/webhook/label tests: 136 passed.
  • All runnable Node test files: 288 passed.
  • UI suite: 44 files / 362 tests passed.
  • Root/core lint, typecheck, and core build passed.
  • Two Redis integration files could not run because Redis and Docker are unavailable in the environment.

Detailed Logs:

  • Conversation: 132 messages
  • Session: 01a00207-f92e-7671-80f8-59b09ffe9c40

Log files stored at:

  • conversation: /tmp/claude-logs/issue-1905-2026-08-14T21-07-56-136Z-conversation.json
  • output: /tmp/claude-logs/issue-1905-2026-08-14T21-07-56-136Z-output.txt
Latest Conversation Messages

This PR was created automatically by ProPR after processing issue #1905.


💡 Need changes?

Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!

Implemented by ProPR AI using gpt-5.6-sol model.

Implementation completed successfully.
@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Please make one narrow correction on this exact head. Replace the new manual target-resolution logic with ProPR’s existing canonical enabled agent/model resolver so /use accepts exactly the supported short aliases and full labels already accepted elsewhere, including configured agent aliases, case-insensitive supported model IDs, and dynamic/hashed full labels. Build the canonical managed label from that resolved agent/model, replace only labels matching MODEL_LABEL_PATTERN, and return. Add focused regressions for those resolution forms. Do not enqueue, reroute, persist command/provider/model data, touch pending comments/retries/task state, or change /switch.

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit 28dab91

Summary of Changes

Implemented the narrow /use correction on the exact PR head.

  • Replaced manual target matching with resolveReviewModels, ProPR’s canonical enabled agent/model resolver.
  • Builds canonical static or dynamic labels from the resolved pair.
  • Preserves scoped MODEL_LABEL_PATTERN replacement behavior.
  • Added regressions for configured aliases, case-insensitive model IDs, dynamic labels, and hashed full labels.
  • Left /switch, queues, persistence, pending comments, retries, and task state untouched.

Validation:

  • Focused suite: 61/61 passed.
  • Hashed-label suite: 3/3 passed.
  • Typecheck passed.
  • No commit created.

🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @integry
  • Time: 5m 17s
  • Tokens: 1,499,007 (1,489,390 in / 9,617 out)
  • Cost: $1.54

Undo ChangesView Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5298204684✓

Implemented the narrow `/use` correction on the exact PR head.

- Replaced manual target matching with `resolveReviewModels`, ProPR’s canonical enabled agent/model resolver.
- Builds canonical static or dynamic labels from the resolved pair.
- Preserves scoped `MODEL_LABEL_PATTERN` replacement behavior.
- Added regressions for configured aliases, case-insensitive model IDs, dynamic labels, and hashed full labels.
- Left `/switch`, queues, persistence, pending comments, retries, and task state untouched.

Validation:

- Focused suite: 61/61 passed.
- Hashed-label suite: 3/3 passed.
- Typecheck passed.
- No commit created.

PR: #1909
Comment by: @integry (ID: 5298204684)
Model: gpt-5.6-sol
@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

/merge

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🔀 Auto-merged main into 1905/gpt-5.6-sol-make-use-change-only-the--20260814-2046-swx (clean merge) in commit 9e33bc7

No conflicts were found — the merge was verified by an AI agent.


🤖 Verification Details

  • Model: gpt-5.6-sol
  • Time: 1m 37s

View Task Execution


System-triggered merge conflict resolution

@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Please make one final narrow label-mutation correction on this exact post-merge head. /use must read current PR labels with the existing live PR lookup for every event type; pull_request_review_comment payload labels can be stale and must not drive replacement. Also preserve a valid managed label on target-add failure: when the canonical target is absent, add it first, abort without removing any existing managed label if that add fails, and only then remove stale managed labels. If the target is already present, remove only stale managed labels. Keep failure logging/no success claim and always return without queue, pending-comment, retry, task-state, or worker changes. Add focused regressions proving a review-comment event uses live rather than stale payload labels, and a failed target add performs no old-label removals; keep idempotent and update-failure cases green.

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit 5899597

Summary of Changes

Implemented the narrow /use correction on the exact PR head.

  • /use now always fetches live PR labels, including review-comment events.
  • Missing targets are added before stale managed labels are removed.
  • Failed target additions abort cleanup, preserving existing managed labels.
  • Target-present behavior removes only stale labels.
  • Failure logging, idempotency, and no-queue/pending-state behavior remain intact.
  • Added focused regressions for stale review payloads and failed-add preservation.

Changed:

Validation:

  • Focused suite: 63/63 passed
  • Typecheck passed
  • git diff --check passed
  • No commit created

🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @integry
  • Time: 5m 7s
  • Tokens: 2,053,429 (2,043,636 in / 9,793 out)
  • Cost: $1.75

Undo ChangesView Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5298323060✓

Implemented the narrow `/use` correction on the exact PR head.

- `/use` now always fetches live PR labels, including review-comment events.
- Missing targets are added before stale managed labels are removed.
- Failed target additions abort cleanup, preserving existing managed labels.
- Target-present behavior removes only stale labels.
- Failure logging, idempotency, and no-queue/pending-state behavior remain intact.
- Added focused regressions for stale review payloads and failed-add preservation.

Changed:

- [commentEventHandler.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T21-25-35/packages/core/src/webhook/commentEventHandler.ts)
- [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T21-25-35/test/commentEventHandler.switch-use.test.ts)

Validation:

- Focused suite: 63/63 passed
- Typecheck passed
- `git diff --check` passed
- No commit created

PR: #1909
Comment by: @integry (ID: 5298323060)
Model: gpt-5.6-sol
@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

/review codex-gpt55

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex-gpt55

Overall Evaluation

This PR is ready to merge. It narrows /use to model-label mutation only, reuses the existing model resolution path, and avoids job enqueueing or pending-comment side effects.

Live PR labels/use now fetches current PR labels before mutation, including for review-comment events.

Safe add-first order — the target label is added before stale managed labels are removed, and add failure aborts removals.

Focused regressions — tests cover stale review-comment payload labels, add failure preservation, aliases, full labels, dynamic labels, idempotency, and no queueing.

Merge blockers

No merge blockers.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Custom Prefix Coverage

The new implementation derives a label prefix from MODEL_LABEL_PATTERN and rewrites canonical llm-* labels accordingly, but the focused tests appear to use the default ^llm-(.+)$ path. A small regression with a custom literal prefix such as ^ai-model-(.+)$ would lock down that intended behavior. This is optional because the main acceptance criteria and default production path are already covered.

Score

The PR satisfies the requested narrow behavior, has current-head checks passing, and includes targeted tests for the recent correction points. The remaining suggestion is optional coverage hardening, not a merge blocker.

Score: 9/10


🤖 Review Details

  • Model: GPT-5.5
  • Time: 2m 35s
  • Tokens: 56,035 (47,975 in / 8,060 out)
  • Cost: $0.48

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

/ultrafix goal=8 max=10

@propr-dev propr-dev Bot added the ultrafix label Aug 14, 2026
@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🔄 Ultrafix loop started (goal: 8/10, max cycles: 10)

First action: /review

💡 Tip: Remove the ultrafix label from this PR to stop further ultrafix cycles.

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This PR correctly redefines /use as a label-only operation and includes strong coverage for aliases, disabled models, live labels, idempotency, and enqueue prevention. It needs two localized correctness fixes before merge despite all 11 current checks passing.

Label-only control flow/use returns before manual takeover, batching, persistence, or job enqueueing.

Careful label selection — The handler reads live PR labels, preserves unrelated labels, and treats an already-active canonical label as an idempotent no-op.

Validated model resolution — Targets must resolve to a supported model on an enabled configured agent before mutation begins.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F1: 🔴 Failed removal leaves ambiguous routing

  • Required behavior: /use must replace the managed model labels without making the PR’s model source of truth incorrect or ambiguous when the replacement fails.
  • Evidence: packages/core/src/webhook/commentEventHandler.ts:401-422 — the target label is committed in one successful update before stale labels are removed in a second update. If removal fails, the handler returns with both the newly added target and old managed labels still present; downstream routing selects the first matching label.
  • Minimum fix: If stale-label removal fails after adding the target, perform a compensating removal of the newly added target, or otherwise restore the original managed-label set before returning. Add a regression test for this failure path.

F2: 🔴 Custom prefixes can exceed GitHub limits

  • Required behavior: A supported target must resolve to a valid canonical model label that can be applied under the configured MODEL_LABEL_PATTERN.
  • Evidence: packages/core/src/webhook/commentEventHandler.ts:341-363 — the label builder limits an llm- label to GitHub’s 50-character maximum, but the code subsequently replaces llm- with an arbitrarily longer derived prefix without reapplying that limit. A 50-character hashed dynamic label therefore becomes invalid with patterns such as ^ai-model-(.+)$.
  • Minimum fix: Build static and dynamic labels using the derived prefix’s actual length budget, preserving the existing stable hash and round-trip behavior, and cover a long dynamic model with a longer custom prefix.

Suggestions

These are optional follow-ups and are not sent to /fix.

No suggestions.

Score

The core behavior and test coverage are strong, but the failure-path routing ambiguity and invalid-label construction for supported custom-prefix configurations must be corrected before merge.

Score: 6/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 2m 36s
  • Tokens: 55,059 (48,261 in / 6,798 out)
  • Cost: $0.40

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit a5bfb27

Addressed 1 AI review comment (IDs: 5298512919)

Summary of Changes

Implemented only F1 and F2, without committing.

  • F1: stale-label removal failure now compensates by removing the newly added target label.
  • F2: canonical labels now use the configured prefix’s actual 50-character budget while retaining stable hashes and routing round trips.
  • Added focused regressions in commentEventHandler.switch-use.test.ts.

Verification passed:

  • 65 targeted tests
  • Repository-wide TypeScript check
  • Core package TypeScript check
  • Package-local ESLint
  • git diff --check

🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @propr-ultrafix
  • Time: 5m 44s
  • Tokens: 1,138,137 (1,126,981 in / 11,156 out)
  • Cost: $1.30

View Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR

Implemented only F1 and F2, without committing.

- F1: stale-label removal failure now compensates by removing the newly added target label.
- F2: canonical labels now use the configured prefix’s actual 50-character budget while retaining stable hashes and routing round trips.
- Added focused regressions in [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T21-51-07/test/commentEventHandler.switch-use.test.ts:732).

Verification passed:

- 65 targeted tests
- Repository-wide TypeScript check
- Core package TypeScript check
- Package-local ESLint
- `git diff --check`

PR: #1909
Comment by: @propr-ultrafix (ID: 0)
Model: gpt-5.6-sol
@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

The PR correctly narrows /use to label-only model selection and has strong happy-path and failure-path coverage. However, three boundary cases can leave /use rejecting valid labels or successfully installing an unusable/missing routing label, so minor corrections are required before merge.

Work remains isolated/use returns before manual takeover, batching, persistence, and enqueue logic; tests verify both the job queue and Redis remain untouched.

Live labels and failures are handled carefully — the implementation reads current PR labels, preserves unrelated labels, validates enabled model support, and avoids success logging when mutations fail.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F3: 🔴 Validate generated label routing

  • Required behavior: The selected label must remain a usable source of truth for subsequent ProPR work; /use must not claim success for an unsupported or ambiguous routing label.
  • Evidence: packages/core/src/webhook/commentEventHandler.ts:328 — the new builder truncates agentAlias, while packages/core/src/webhook/commentEventHandler.ts:399 only verifies that the resulting label matches MODEL_LABEL_PATTERN. Two long aliases sharing the visible truncated prefix can therefore produce an ambiguous token that cannot resolve back to the selected agent/model.
  • Minimum fix: Extract the canonical label’s routing capture, resolve it through the normal model resolver, and require an exact agent/model match before mutating labels. Add a regression test with colliding truncated agent aliases.

F4: 🔴 Accept configured-prefix full labels

  • Required behavior: /use <model-or-label> must accept supported full model labels, including labels managed through the configured MODEL_LABEL_PATTERN.
  • Evidence: packages/core/src/webhook/commentEventHandler.ts:425 — the handler reads the configured pattern but passes commandMeta.models[0] directly to resolveCanonicalModelLabel at line 426. Consequently, a label it generated under a custom prefix, such as ai-model-codex-gpt56-sol, is submitted to model resolution with that prefix intact and rejected.
  • Minimum fix: When the argument itself matches MODEL_LABEL_PATTERN, resolve its captured routing token rather than the full prefixed label. Cover copying an active custom-prefix label into a repeated /use command.

F5: 🔴 Compare labels case-insensitively

  • Required behavior: Selecting the already-active model must be an idempotent no-op and must not remove the model label.
  • Evidence: packages/core/src/webhook/commentEventHandler.ts:431 — existing labels are compared to the canonical label with case-sensitive !== and includes; lines 443–454 then add the canonical spelling before removing the differently-cased existing spelling. Because GitHub label identity is case-insensitive, adding llm-claude-opus5 when llm-Claude-Opus5 exists can succeed as a no-op and the subsequent removal deletes the only label.
  • Minimum fix: Compare managed label names using GitHub’s case-insensitive identity semantics and never schedule an equivalent existing target for removal. Add a mixed-case existing-label test.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Restore partial removals

If removal of several stale labels partially succeeds, the failure path removes the newly added target but does not restore labels listed in removalUpdate.removed. Re-adding those labels would improve rollback fidelity. This is optional because the stated failure contract only requires avoiding success claims and enqueued work, which the current implementation satisfies.

S2: 🟢 Centralize prefixed label building

buildPrefixedDynamicModelLabel duplicates much of the shared dynamic-label length and hashing algorithm. Moving custom-prefix support into the shared label utility would reduce future drift, but this is maintainability work rather than a merge requirement.

Score

The principal behavior and tests are solid, and all current checks pass, but the three routing and label-identity edge cases can produce incorrect successful outcomes. The fixes are localized.

Score: 6/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 4m 57s
  • Tokens: 67,172 (54,088 in / 13,084 out)
  • Cost: $0.62

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit 133f79a

Addressed 1 AI review comment (IDs: 5298671966)

Summary of Changes

Implemented F3, F4, and F5 only.

  • Validates generated routing tokens resolve to the exact selected agent/model.
  • Supports full labels using custom MODEL_LABEL_PATTERN prefixes.
  • Compares GitHub labels case-insensitively to preserve idempotency.
  • Added regressions for truncated alias collisions, custom-prefix labels, and mixed-case labels.

Verification passed:

  • Focused suite: 68 tests
  • TypeScript typecheck
  • Core ESLint
  • git diff --check

No commit created.


🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @propr-ultrafix
  • Time: 5m 13s
  • Tokens: 813,187 (804,469 in / 8,718 out)
  • Cost: $0.97

View Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR

@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Every PR can have just a single LLM label at the same time, there can be no ambiguity. Just change the fucking label.

Implemented F3, F4, and F5 only.

- Validates generated routing tokens resolve to the exact selected agent/model.
- Supports full labels using custom `MODEL_LABEL_PATTERN` prefixes.
- Compares GitHub labels case-insensitively to preserve idempotency.
- Added regressions for truncated alias collisions, custom-prefix labels, and mixed-case labels.

Verification passed:

- Focused suite: 68 tests
- TypeScript typecheck
- Core ESLint
- `git diff --check`

No commit created.

PR: #1909
Comment by: @propr-ultrafix (ID: 0)
Model: gpt-5.6-sol
@integry integry added the AI label Aug 14, 2026
@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit 30651b3

Summary of Changes

Implemented the follow-up correction.

  • /use now replaces the live PR label set in one request, preserving unrelated labels and leaving exactly one managed LLM label.
  • Failed replacement leaves existing labels untouched and does not claim success or enqueue work.
  • Updated focused regressions in commentEventHandler.switch-use.test.ts.

Validation passed:

  • 68/68 focused tests
  • TypeScript typecheck
  • Core ESLint
  • git diff --check

No commit created.


🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @integry
  • Time: 7m 54s
  • Tokens: 2,763,104 (2,747,988 in / 15,116 out)
  • Cost: $2.45

Undo ChangesView Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5298715670✓

Implemented the follow-up correction.

- `/use` now replaces the live PR label set in one request, preserving unrelated labels and leaving exactly one managed LLM label.
- Failed replacement leaves existing labels untouched and does not claim success or enqueue work.
- Updated focused regressions in [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T22-22-00/test/commentEventHandler.switch-use.test.ts).

Validation passed:

- 68/68 focused tests
- TypeScript typecheck
- Core ESLint
- `git diff --check`

No commit created.

PR: #1909
Comment by: @integry (ID: 5298715670)
Model: gpt-5.6-sol
@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

/review

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This PR correctly reframes /use as a label-only command and adds strong resolution and no-queue coverage. It needs one narrow correctness change before merge: the replace-all label request can overwrite concurrent unrelated-label changes. The full test suite is also still pending.

Label-only control flow/use returns before takeover, queue, pending-comment, retry, and task-state paths, with focused tests covering active and inactive jobs.

Canonical live resolution — The handler uses resolveReviewModels, validates the generated label by routing it back, and reads live PR labels for every event type rather than trusting review-comment payload labels.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F6: 🔴 Replace-all update can lose unrelated labels

  • Required behavior: /use must preserve every unrelated label while replacing only labels matching MODEL_LABEL_PATTERN; changed behavior must not overwrite concurrent unrelated-label updates.
  • Evidence: packages/core/src/webhook/commentEventHandler.ts:463 — the handler reads a label snapshot, reconstructs the complete label collection at lines 475–478, and sends it through the replace-all PUT /issues/{issue_number}/labels request at lines 480–488. An unrelated label added or removed after the GET can therefore be dropped or restored by this stale replacement.
  • Minimum fix: Avoid submitting unrelated labels. If the canonical target is absent, add it first and abort without removing stale managed labels if that addition fails; then remove only the stale managed labels. If the target is present, remove only stale managed labels. Retain failure logging and add a regression proving concurrent unrelated-label state is untouched.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Centralize prefixed label construction

buildPrefixedDynamicModelLabel duplicates the hashing, sanitization, and length-budget logic from the shared label utilities. Moving custom-prefix construction into the shared utility would reduce future format drift, but the current round-trip validation makes this optional for the present PR.

Score

The core control flow, resolution behavior, and regression coverage are strong, but the label replacement strategy can violate a central preservation requirement. The remaining correction is localized; the pending full-suite check should complete afterward.

Score: 6/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 2m 51s
  • Tokens: 56,896 (49,663 in / 7,233 out)
  • Cost: $0.42

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Any model specific labels are prefixed via "llm-" so the operation is simple - remove all llm- prefixed labels and add the new label.

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-up changes in commit d869904

Summary of Changes

Implemented the narrow /use correction:

  • Removes every live PR label prefixed with llm-.
  • Adds the resolved canonical target once.
  • Preserves unrelated labels and idempotent behavior.
  • Added focused regression coverage.

Files changed:

Validation passed: 69 focused tests, TypeScript typecheck, core ESLint, and git diff --check. No commit created.


🤖 Implementation Details

  • Model: GPT-5.6 Sol (high)
  • Requested By: @integry
  • Time: 4m 9s
  • Tokens: 958,530 (951,635 in / 6,895 out)
  • Cost: $1.03

Undo ChangesView Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5298894213✓

Implemented the narrow `/use` correction:

- Removes every live PR label prefixed with `llm-`.
- Adds the resolved canonical target once.
- Preserves unrelated labels and idempotent behavior.
- Added focused regression coverage.

Files changed:

- [commentEventHandler.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T22-45-04/packages/core/src/webhook/commentEventHandler.ts:459)
- [commentEventHandler.switch-use.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-1909-followup-2026-08-14T22-45-04/test/commentEventHandler.switch-use.test.ts:629)

Validation passed: 69 focused tests, TypeScript typecheck, core ESLint, and `git diff --check`. No commit created.

PR: #1909
Comment by: @integry (ID: 5298894213)
Model: gpt-5.6-sol
@integry

integry commented Aug 14, 2026

Copy link
Copy Markdown
Owner

/review

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

🔍 AI Code Review — codex:gpt-5.6-sol

Overall Evaluation

This PR correctly narrows /use to model-label mutation and provides strong resolver and no-enqueue regression coverage. However, two label-preservation issues remain, so it needs minor changes before merge.

No work is queued/use returns before takeover, batching, persistence, and enqueue paths, with focused tests covering active jobs and trailing instructions.

Live labels are consulted — the handler calls the live PR lookup instead of relying on potentially stale review-comment payload labels.

Canonical resolution is well covered — tests exercise short aliases, configured agent aliases, case-insensitive IDs, and dynamic/hashed labels.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F7: 🔴 Managed labels ignore configured pattern

  • Required behavior: /use must replace labels matching MODEL_LABEL_PATTERN while preserving unrelated labels.
  • Evidence: packages/core/src/webhook/commentEventHandler.ts:462 — managed labels are identified with the hard-coded, case-sensitive startsWith('llm-'); packages/core/src/webhook/commentEventHandler.ts:473 similarly removes every llm-* label rather than labels matching the configured pattern. With ^ai-model-(.+)$, stale ai-model-* labels remain while unrelated llm-* labels can be removed.
  • Minimum fix: Use the compiled MODEL_LABEL_PATTERN to identify stale managed labels throughout handleUseCommand, removing only matching labels other than the canonical target.

F8: 🔴 Whole-label replacement permits lost updates

  • Required behavior: Unrelated PR labels must be preserved, and changing the model must not discard the current managed label when adding the target fails.
  • Evidence: packages/core/src/webhook/commentEventHandler.ts:460 reads a label snapshot, and packages/core/src/webhook/commentEventHandler.ts:480 replaces the complete label set with PUT. An unrelated label added or removed between those calls is overwritten, and the operation does not implement the required add-target-before-removing-stale ordering.
  • Minimum fix: If the target is absent, add it first and return without removals on failure. After a successful add—or when it is already present—remove only stale managed labels individually, leaving unrelated labels untouched.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Centralize prefixed label construction

The new buildPrefixedDynamicModelLabel and applyModelLabelPrefix functions duplicate much of the shared canonical-label length, sanitization, and hashing logic. Moving prefix-aware construction into the shared label utility would reduce future drift, but the current round-trip validation makes this optional for the present PR.

Score

The resolution and no-enqueue behavior are well tested and all current checks pass, but the two label-mutation defects can retain stale managed labels or lose concurrent unrelated-label changes.

Score: 6/10


🤖 Review Details

  • Model: GPT-5.6 Sol
  • Time: 2m 28s
  • Tokens: 56,185 (50,879 in / 5,306 out)
  • Cost: $0.37

View Task


💡 Next step: Comment /fix to address F# merge blockers only.
F# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. Suggestions require a separate ordinary follow-up request.

🤖 Review by ProPR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make /use change only the PR model label

1 participant