Apply same-file review suggestions as one atomic mobile fix #1385

Merged
rockachopa merged 1 commits from timmy/1384-atomic-review-suggestion-batch into main 2026-08-25 07:26:01 +00:00
8 changed files with 392 additions and 2 deletions

View File

@ -18,7 +18,13 @@ token that can read dashboard data, update the authenticated user's notification
threads, create and self-assign issues, discover, claim, and release issue assignments,
list repository labels and open milestones, set or clear due dates on assigned issues, create issue comments, close assigned issues,
inspect/comment on assigned pull
requests, merge assigned pull requests, and submit pull-request reviews.
requests, merge assigned pull requests, and submit pull-request reviews. For authored
pulls, several non-overlapping one-line reviewer suggestions in the same bounded UTF-8
file can be staged during the mobile feedback pass, reviewed together, and committed as
one race-guarded branch update. The batch preserves the original blob identity, applies
replacements from the bottom up, rejects overlapping or stale lines, and verifies the
advanced pull head and final file before reporting success; suggestions in another file
wait for the next batch so Gitea's single-file contents API cannot produce partial commits.
After an exact merged commit fails release checks, its mobile release receipt can load the
failed job evidence and prepare a draft rollback pull request. The server revalidates the
operator's participation and push access, reverses only bounded UTF-8 files whose current

View File

@ -774,6 +774,12 @@ textarea { resize: vertical; min-height: 120px; }
.pull-feedback-suggestion-change > div { min-width:0; }
.pull-feedback-suggestion-change pre { box-sizing:border-box; max-width:100%; max-height:160px; white-space:pre-wrap; overflow-wrap:anywhere; word-break:break-word; }
.pull-feedback-suggestion-preview input { box-sizing:border-box; width:100%; min-height:44px; }
.pull-feedback-batch-review { min-width:0; display:grid; gap:10px; margin:10px 0; padding:10px; border:1px solid #3b82f6; border-radius:10px; background:#07101e; }
.pull-feedback-batch-review[hidden] { display:none; }
.pull-feedback-batch-review h5 { margin:0; }
.pull-feedback-batch-review ul { min-width:0; margin:0; padding-left:20px; overflow-wrap:anywhere; }
.pull-feedback-batch-review input { box-sizing:border-box; width:100%; min-width:0; min-height:44px; }
#review-pull-feedback-batch { position:sticky; bottom:calc(8px + env(safe-area-inset-bottom)); z-index:4; width:100%; min-height:44px; }
.review-handoff { position:sticky; bottom:0; z-index:3; display:grid; gap:8px; padding:10px 4px; padding-bottom:calc(10px + env(safe-area-inset-bottom)); background:rgba(11,21,38,.98); border-top:1px solid #2a496e; }
.review-handoff button { min-height:44px; }
.review-handoff-link { min-height:44px; display:flex; align-items:center; justify-content:center; border:1px solid #60a5fa; border-radius:8px; color:#bfdbfe; font-weight:700; text-decoration:none; }

View File

@ -1755,9 +1755,21 @@
<input id="pull-feedback-suggestion-message" maxlength="120" value="fix: apply review suggestion">
<div class="pull-feedback-file-actions">
<button id="cancel-pull-feedback-suggestion" type="button" hidden>Cancel suggestion</button>
<button id="stage-pull-feedback-suggestion" type="button" hidden>Add to fix batch</button>
<button id="apply-pull-feedback-suggestion" type="button" hidden>Commit suggested change</button>
</div>
</section>
<section class="pull-feedback-batch-review" id="pull-feedback-batch-review" aria-labelledby="pull-feedback-batch-heading" hidden>
<h5 id="pull-feedback-batch-heading">Review fixes</h5>
<ul id="pull-feedback-batch-items"></ul>
<label for="pull-feedback-batch-message">Commit message</label>
<input id="pull-feedback-batch-message" maxlength="120" value="fix: apply review suggestions">
<div class="pull-feedback-file-actions">
<button id="cancel-pull-feedback-batch" type="button" hidden>Keep reviewing</button>
<button id="commit-pull-feedback-batch" type="button" hidden>Commit all fixes</button>
</div>
</section>
<button id="review-pull-feedback-batch" type="button" hidden>Review fixes (0)</button>
<section class="pull-feedback-file-editor" id="pull-feedback-file-editor" hidden>
<h5>Edit <span id="pull-feedback-file-path"></span></h5>
<label for="pull-feedback-file-content">File content</label>

View File

@ -416,6 +416,7 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
const clearSuggestion = () => {
qs('#pull-feedback-suggestion-preview').hidden = true;
qs('#cancel-pull-feedback-suggestion').hidden = true;
qs('#stage-pull-feedback-suggestion').hidden = true;
qs('#apply-pull-feedback-suggestion').hidden = true;
if (active) active.suggestion = null;
};
@ -430,6 +431,25 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
lines.splice(line - 1, 1, ...suggestion.content.split('\n'));
return { before, after:suggestion.content, content:lines.join('\n') + (trailingNewline ? '\n' : '') };
};
const renderBatchButton = () => {
const count = active?.batch ? Object.values(active.batch)
.reduce((total, file) => total + file.suggestions.length, 0) : 0;
const button = qs('#review-pull-feedback-batch');
button.hidden = count < 2;
button.textContent = 'Review fixes (' + count + ')';
};
const rebuildBatchFile = entry => {
const trailingNewline = entry.file.content.endsWith('\n');
const lines = entry.file.content.split('\n');
if (trailingNewline) lines.pop();
const ordered = [...entry.suggestions].sort((left, right) => right.line - left.line);
if (new Set(ordered.map(item => item.line)).size !== ordered.length) return null;
for (const suggestion of ordered) {
if (suggestion.line < 1 || suggestion.line > lines.length) return null;
lines.splice(suggestion.line - 1, 1, ...suggestion.content.split('\n'));
}
return lines.join('\n') + (trailingNewline ? '\n' : '');
};
const render = () => {
if (!active) return;
const items = comments();
@ -500,15 +520,17 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
const reviewer = (detail?.reviewers || []).find(candidate => candidate.review_id === reviewId);
if (!detail || !item || !reviewer) return;
const login = getLogin?.() || '';
active = { item, detail, reviewer, login,
active = { item, detail, reviewer, login, batch:{},
state:controller.loadFeedbackPass(item, detail, reviewer, login) };
panel.hidden = false;
qs('#pull-feedback-file-editor').hidden = true;
qs('#pull-feedback-suggestion-preview').hidden = true;
qs('#pull-feedback-batch-review').hidden = true;
qs('#cancel-pull-feedback-fix').hidden = true;
qs('#commit-pull-feedback-fix').hidden = true;
active.file = null;
active.suggestion = null;
renderBatchButton();
qs('#pull-feedback-status').textContent = active.state.posted ?
'Response already posted. You can request an updated review.' : 'Progress saves on this device.';
render();
@ -534,6 +556,7 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
qs('#pull-feedback-file-editor').hidden = true;
qs('#pull-feedback-suggestion-preview').hidden = false;
qs('#cancel-pull-feedback-suggestion').hidden = false;
qs('#stage-pull-feedback-suggestion').hidden = false;
qs('#apply-pull-feedback-suggestion').hidden = false;
qs('#pull-feedback-status').textContent = 'Review the bounded replacement before committing.';
qs('#apply-pull-feedback-suggestion').focus();
@ -548,6 +571,90 @@ function bindFeedbackControls(doc, controller, getSelected, getDetail, getLogin)
clearSuggestion();
qs('#review-pull-feedback-suggestion').focus();
});
qs('#stage-pull-feedback-suggestion').addEventListener('click', () => {
if (!active?.suggestion) return;
const pending = active.suggestion;
const path = pending.file.path;
const stagedPath = Object.keys(active.batch)[0];
if (stagedPath && stagedPath !== path) {
qs('#pull-feedback-status').textContent = 'Commit the staged file before batching suggestions from another file.';
return;
}
const entry = active.batch[path] || {file:pending.file, suggestions:[]};
if (entry.file.blob_sha !== pending.file.blob_sha ||
entry.suggestions.some(item => item.commentId === pending.commentId)) return;
const comment = current();
entry.suggestions.push({
line:Number(comment.suggestion.line), content:comment.suggestion.content,
commentId:pending.commentId,
});
entry.content = rebuildBatchFile(entry);
if (entry.content === null) {
entry.suggestions.pop();
qs('#pull-feedback-status').textContent = 'This suggestion overlaps a staged fix. Keep one of the conflicting changes.';
return;
}
active.batch[path] = entry;
clearSuggestion();
renderBatchButton();
qs('#pull-feedback-status').textContent = 'Suggestion added to the fix batch. Keep reviewing this feedback.';
qs('#next-pull-feedback').focus();
});
qs('#review-pull-feedback-batch').addEventListener('click', () => {
if (!active?.batch) return;
const list = qs('#pull-feedback-batch-items');
list.replaceChildren(...Object.values(active.batch).map(entry => {
const item = doc.createElement('li');
item.textContent = entry.file.path + ' · ' + entry.suggestions.length +
(entry.suggestions.length === 1 ? ' suggestion' : ' suggestions');
return item;
}));
qs('#pull-feedback-batch-review').hidden = false;
qs('#cancel-pull-feedback-batch').hidden = false;
qs('#commit-pull-feedback-batch').hidden = false;
qs('#pull-feedback-batch-review').scrollIntoView({block:'center', behavior:'smooth'});
qs('#commit-pull-feedback-batch').focus();
});
qs('#cancel-pull-feedback-batch').addEventListener('click', () => {
qs('#pull-feedback-batch-review').hidden = true;
qs('#cancel-pull-feedback-batch').hidden = true;
qs('#commit-pull-feedback-batch').hidden = true;
qs('#review-pull-feedback-batch').focus();
});
qs('#commit-pull-feedback-batch').addEventListener('click', async () => {
if (!active?.batch) return;
const entries = Object.values(active.batch);
const count = entries.reduce((total, entry) => total + entry.suggestions.length, 0);
const message = qs('#pull-feedback-batch-message').value.trim();
if (count < 2 || !message) return;
if (!globalThis.confirm('Commit ' + count + ' suggested changes to ' + active.item.key + '?')) return;
const button = qs('#commit-pull-feedback-batch');
button.disabled = true;
qs('#pull-feedback-status').textContent = 'Committing and verifying all suggested changes…';
try {
const result = await controller.commitFeedbackBatch(active.item, {
file:{
path:entries[0].file.path, content:entries[0].content,
expectedBlobSha:entries[0].file.blob_sha,
},
suggestionCount:count, message, expectedHeadSha:active.detail.head_sha,
});
active.detail.head_sha = result.head_sha;
active.state.headSha = result.head_sha;
active.batch = {};
save();
qs('#pull-feedback-batch-review').hidden = true;
qs('#cancel-pull-feedback-batch').hidden = true;
qs('#commit-pull-feedback-batch').hidden = true;
renderBatchButton();
qs('#pull-feedback-status').textContent = count + ' suggested changes committed. Mark each comment Addressed when satisfied.';
render();
} catch (error) {
qs('#pull-feedback-status').textContent = error.message + ' Batch kept; reload if the branch changed, or retry.';
button.disabled = false;
button.focus();
}
});
qs('#apply-pull-feedback-suggestion').addEventListener('click', async () => {
if (!active?.suggestion) return;
const message = qs('#pull-feedback-suggestion-message').value.trim();
@ -951,6 +1058,28 @@ function createPullSheet({ fetchJson, storage, onState, createConversationPager
}).finally(() => { feedbackFixRequest = null; });
return feedbackFixRequest;
},
commitFeedbackBatch(item, draft) {
if (feedbackFixRequest) return feedbackFixRequest;
feedbackFixRequest = fetchJson(pathFor(item) + '/feedback-batch', {
method:'POST',
headers:{ Accept:'application/json', 'Content-Type':'application/json' },
body:JSON.stringify({
file:{
path:draft.file.path, content:draft.file.content,
expected_blob_sha:draft.file.expectedBlobSha,
},
suggestion_count:draft.suggestionCount,
message:draft.message, expected_head_sha:draft.expectedHeadSha,
}),
}).then(result => {
if (!result?.head_sha || result.head_sha === draft.expectedHeadSha ||
result.path !== draft.file.path || result.suggestion_count !== draft.suggestionCount) {
throw new Error('The suggestion batch was not confirmed.');
}
return result;
}).finally(() => { feedbackFixRequest = null; });
return feedbackFixRequest;
},
loadChecks(item) {
if (checkRequest) return checkRequest;
checkRequest = fetchJson(pathFor(item) + '/checks', {

View File

@ -2419,6 +2419,24 @@ async def commit_authored_pull_feedback_fix(
}
async def commit_authored_pull_feedback_batch(
repository: str,
number: int,
file: dict,
suggestion_count: int,
message: str,
expected_head_sha: str,
) -> dict:
"""Commit multiple reviewed replacements in one file as one verified commit."""
if not 2 <= suggestion_count <= 8:
raise IssueNotAvailableError("Feedback batch is outside the editable bounds")
result = await commit_authored_pull_feedback_fix(
repository, number, file.get("path", ""), file.get("content", ""), message,
expected_head_sha, file.get("expected_blob_sha", ""),
)
return {**result, "suggestion_count": suggestion_count}
async def update_authored_pull_branch(
repository: str, number: int, expected_head_sha: str
) -> dict:

View File

@ -1166,6 +1166,21 @@ class PullFeedbackFileUpdate(BaseModel):
expected_blob_sha: str = Field(min_length=1, max_length=128)
class PullFeedbackBatchFile(BaseModel):
path: str = Field(min_length=1, max_length=1024)
content: str = Field(max_length=131_072)
expected_blob_sha: str = Field(min_length=1, max_length=128)
class PullFeedbackBatchUpdate(BaseModel):
file: PullFeedbackBatchFile
suggestion_count: int = Field(ge=2, le=8)
message: str = Field(min_length=1, max_length=120)
expected_head_sha: str = Field(
min_length=7, max_length=64, pattern=r"^[A-Fa-f0-9]+$"
)
class PullContentUpdate(BaseModel):
title: str = Field(min_length=1, max_length=255)
body: str = Field(default="", max_length=10_000)
@ -7159,6 +7174,34 @@ async def commit_authored_pull_feedback_fix(
return JSONResponse(result)
@app.post("/api/v1/repos/{owner}/{repo}/pulls/{number}/feedback-batch")
async def commit_authored_pull_feedback_batch(
update: PullFeedbackBatchUpdate,
owner: str,
repo: str,
number: int = PathParam(gt=0),
):
try:
result = await asyncio.wait_for(
gitea_proxy.commit_authored_pull_feedback_batch(
f"{owner}/{repo}", number, update.file.model_dump(),
update.suggestion_count, update.message, update.expected_head_sha,
),
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
)
except gitea_proxy.IssueNotAvailableError:
return JSONResponse(
{"error": "The pull request or a suggested file changed. No fixes were committed."},
status_code=409,
)
except Exception:
return JSONResponse(
{"error": "The suggestion batch could not be confirmed. Keep the batch and retry."},
status_code=503, headers={"Retry-After": "1"},
)
return JSONResponse(result)
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review-data")
async def assigned_pull_review_data(
owner: str, repo: str, number: int = PathParam(gt=0)

View File

@ -0,0 +1,108 @@
from pathlib import Path
import pytest
ROOT = Path(__file__).parents[2]
@pytest.mark.parametrize("viewport", [(320, 568), (390, 844)])
def test_mobile_author_stages_and_commits_two_same_file_suggestions_atomically(viewport):
playwright = pytest.importorskip("playwright.sync_api")
html = (ROOT / "frontend" / "index.html").read_text()
detail = {
"state": "open", "merged": False, "head_sha": "abc1234",
"capabilities": {"authored": True},
"reviewers": [{
"review_id": 42, "login": "sam", "status": "changes_requested",
"head_sha": "abc1234", "blocking": True, "feedback_state": "loaded",
"comments": [
{"id": 99, "path": "src/api.py", "line": 2, "body": "Use the typed empty result.",
"suggestion": {"line": 2, "content": " return empty_result()"}},
{"id": 100, "path": "src/api.py", "line": 3, "body": "Keep the result typed.",
"suggestion": {"line": 3, "content": " return typed_result()"}},
],
}],
"files": [
{"filename": "src/api.py", "status": "modified", "diff_available": True,
"diff_lines": ["@@ -2 +2 @@", "+ return None"]},
],
}
with playwright.sync_playwright() as runtime:
try:
browser = runtime.chromium.launch(headless=True)
except Exception as error:
pytest.skip(f"Chromium is not installed: {error}")
page = browser.new_page(viewport={"width": viewport[0], "height": viewport[1]})
page.set_content(html, wait_until="domcontentloaded")
page.add_style_tag(path=ROOT / "frontend" / "dashboard.css")
page.add_script_tag(path=ROOT / "frontend" / "pull-sheet.js")
page.evaluate(
"""detail => {
globalThis.confirm = () => true;
globalThis.batchCalls = [];
globalThis.batchItem = {repository:'stackchain/api', number:7, key:'stackchain/api#7'};
globalThis.batchDetail = detail;
globalThis.batchController = createPullSheet({fetchJson: async (path, options = {}) => {
batchCalls.push({path, options});
if ((options.method || 'GET') === 'GET') {
return {path:'src/api.py', head_sha:'abc1234', blob_sha:'blob1',
content:'def lookup():\\n return None\\n return result\\n'};
}
return {path:'src/api.py', suggestion_count:2, previous_head_sha:'abc1234', head_sha:'def5678'};
}});
document.querySelector('#pull-sheet').classList.add('open');
document.querySelector('#pull-review').open = true;
document.querySelector('#pull-files').innerHTML = detail.files.map((file, index) =>
createPullSheet.renderFile(file, index, false, value => String(value))
).join('');
createPullSheet.bindFeedbackControls(document, batchController,
() => batchItem, () => batchDetail, () => 'alex');
createPullSheet.review(detail, null, document);
}""",
detail,
)
page.locator(".pull-review-feedback summary").click()
page.locator('[data-address-review-feedback="42"]').click()
page.locator("#review-pull-feedback-suggestion").click()
page.wait_for_function("batchCalls.length === 1")
page.locator("#stage-pull-feedback-suggestion").click()
assert page.locator("#review-pull-feedback-batch").text_content() == "Review fixes (1)"
page.locator("#next-pull-feedback").click()
page.locator("#review-pull-feedback-suggestion").click()
page.wait_for_function("batchCalls.length === 2")
page.locator("#stage-pull-feedback-suggestion").click()
assert page.locator("#review-pull-feedback-batch").text_content() == "Review fixes (2)"
page.locator("#review-pull-feedback-batch").click()
assert page.locator("#pull-feedback-batch-review").is_visible()
assert page.locator("#pull-feedback-batch-items li").count() == 1
page.locator("#commit-pull-feedback-batch").click()
page.wait_for_function("batchCalls.length === 3")
result = page.evaluate("""() => ({
calls:batchCalls, head:batchDetail.head_sha,
reviewHidden:document.querySelector('#pull-feedback-batch-review').hidden,
status:document.querySelector('#pull-feedback-status').textContent,
scrollWidth:document.documentElement.scrollWidth,
clientWidth:document.documentElement.clientWidth,
heights:Array.from(document.querySelectorAll('#pull-feedback-pass button'))
.filter(button => button.getClientRects().length > 0).map(button => button.getBoundingClientRect().height),
})""")
browser.close()
assert result["scrollWidth"] <= result["clientWidth"]
assert min(result["heights"]) >= 44
assert result["head"] == "def5678"
assert result["reviewHidden"] is True
assert "2 suggested changes committed" in result["status"].lower()
request = result["calls"][2]
assert request["path"].endswith("/feedback-batch")
assert request["options"]["method"] == "POST"
body = request["options"]["body"]
assert '"expected_head_sha":"abc1234"' in body
assert '"file":{"path":"src/api.py"' in body
assert '"suggestion_count":2' in body
assert 'return empty_result()' in body
assert 'return typed_result()' in body

View File

@ -108,6 +108,74 @@ async def test_gitea_commits_feedback_fix_and_verifies_advanced_pull_head():
]
@pytest.mark.anyio
async def test_gitea_commits_multiple_same_file_suggestions_as_one_commit():
requests = []
pull = {
"number": 7, "state": "open", "merged": False,
"user": {"login": "alex"},
"head": {"sha": "abc1234", "ref": "alex/review-fix", "repo": {"full_name": "stackchain/api"}},
"base": {"repo": {"full_name": "stackchain/api"}},
}
async def handler(request):
requests.append((request.method, request.url.path))
if request.url.path == "/api/v1/user":
return httpx.Response(200, json={"login": "alex"})
if request.url.path == "/api/v1/repos/stackchain/api/pulls/7":
return httpx.Response(200, json=pull)
if request.url.path == "/api/v1/repos/stackchain/api/contents/src/api.py" and request.method == "GET":
updated = request.url.params["ref"] == "def5678"
return httpx.Response(200, json=_content_payload(
"first fixed\nsecond fixed\n" if updated else "first\nsecond\n",
"blob2" if updated else "blob1",
))
if request.url.path == "/api/v1/repos/stackchain/api/contents/src/api.py" and request.method == "PUT":
body = json.loads(request.content)
assert body["branch"] == "alex/review-fix"
assert base64.b64decode(body["content"]).decode() == "first fixed\nsecond fixed\n"
pull["head"]["sha"] = "def5678"
return httpx.Response(200, json={"commit": {"sha": "def5678"}})
raise AssertionError(f"unexpected request: {request.method} {request.url}")
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
try:
result = await gitea_proxy.commit_authored_pull_feedback_batch(
"stackchain/api", 7,
{"path": "src/api.py", "content": "first fixed\nsecond fixed\n", "expected_blob_sha": "blob1"},
2, "fix: apply review suggestions", "abc1234",
)
finally:
await gitea_proxy.stop_client()
assert result["previous_head_sha"] == "abc1234"
assert result["head_sha"] == "def5678"
assert result["path"] == "src/api.py"
assert result["suggestion_count"] == 2
assert len([request for request in requests if request[0] == "PUT"]) == 1
@pytest.mark.anyio
async def test_feedback_batch_api_requires_at_least_two_suggestions(monkeypatch):
called = False
async def commit(*args):
nonlocal called
called = True
monkeypatch.setattr(main.gitea_proxy, "commit_authored_pull_feedback_batch", commit, raising=False)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
response = await client.post(
"/api/v1/repos/stackchain/api/pulls/7/feedback-batch",
json={"message": "fix: suggestions", "expected_head_sha": "abc1234", "suggestion_count": 1,
"file": {"path": "src/api.py", "content": "one\n", "expected_blob_sha": "blob1"}},
)
assert response.status_code == 422
assert called is False
@pytest.mark.anyio
async def test_feedback_file_api_loads_and_commits_an_authored_pull_fix(monkeypatch):
calls = []