feat: explain Following changes before review (Closes #1322)
All checks were successful
CI / lint (pull_request) Successful in 4m4s
CI / build-release (pull_request) Successful in 7s
CI / browser-journey (pull_request) Successful in 5m48s
CI / release-candidate (pull_request) Has been skipped

This commit is contained in:
timmy 2026-08-23 21:28:50 +00:00
parent 31700d5c71
commit 5921027e80
8 changed files with 174 additions and 18 deletions

View File

@ -118,7 +118,10 @@ Following is a read-first, account-scoped collection: it is encrypted at rest, r
pull-request Search Preview; confirmed **Stop watching** removes an open item. When watched work pull-request Search Preview; confirmed **Stop watching** removes an open item. When watched work
closes or merges, the sequential review exposes **Stop watching & next** so the completed item can be closes or merges, the sequential review exposes **Stop watching & next** so the completed item can be
retired without leaving the preview; the next captured change opens immediately, and retiring the retired without leaving the preview; the next captured change opens immediately, and retiring the
final item completes the Following phase. For open work that still needs thought, **Keep for later & next** final item completes the Following phase. Changed cards explain whether work closed, reopened, changed title, or
received other activity. During review, conversation messages newer than the prior reviewed revision are highlighted,
and the revision is acknowledged only after both detail and conversation context load; a failed conversation load keeps
that exact change unseen for Retry or **Keep for later & next**. For open work that still needs thought, **Keep for later & next**
restores only the loaded revision to the unseen queue and continues the captured pass without changing restores only the loaded revision to the unseen queue and continues the captured pass without changing
Gitea state, ownership, planning, or watch status. Failed or unconfirmed Gitea mutations leave the collection Gitea state, ownership, planning, or watch status. Failed or unconfirmed Gitea mutations leave the collection
and current review position unchanged. Following counts never influence the recommended Work queue. Set and current review position unchanged. Following counts never influence the recommended Work queue. Set

View File

@ -1040,6 +1040,7 @@ textarea { resize: vertical; min-height: 120px; }
.search-preview-conversation { display:grid; gap:8px; padding-top:8px; border-top:1px solid #2a496e; } .search-preview-conversation { display:grid; gap:8px; padding-top:8px; border-top:1px solid #2a496e; }
.search-preview-conversation h2 { margin:0; font-size:1rem; } .search-preview-conversation h2 { margin:0; font-size:1rem; }
.search-preview-comment { min-width:0; padding:10px 0; border-bottom:1px solid #1b2d45; overflow-wrap:anywhere; } .search-preview-comment { min-width:0; padding:10px 0; border-bottom:1px solid #1b2d45; overflow-wrap:anywhere; }
.search-preview-comment.new-since-review { padding-left:10px; border-left:3px solid var(--accent); background:#10233a; }
.search-preview-conversation button { min-height:44px; width:100%; } .search-preview-conversation button { min-height:44px; width:100%; }
.search-preview-reply { display:grid; gap:8px; padding-top:8px; border-top:1px solid #2a496e; } .search-preview-reply { display:grid; gap:8px; padding-top:8px; border-top:1px solid #2a496e; }
.search-preview-reply[hidden] { display:none; } .search-preview-reply[hidden] { display:none; }

View File

@ -184,7 +184,7 @@
list.innerHTML = state.items.map((item, index) => list.innerHTML = state.items.map((item, index) =>
'<button class="following-card' + (item.has_unseen_change ? ' has-unseen-change' : '') + '<button class="following-card' + (item.has_unseen_change ? ' has-unseen-change' : '') +
'" type="button" data-following-index="' + index + '"><span>' + '" type="button" data-following-index="' + index + '"><span>' +
(item.has_unseen_change ? '<em>New activity</em>' : '') + '<strong>' + (item.has_unseen_change ? '<em>' + escapeHtml(item.change_summary || 'New activity') + '</em>' : '') + '<strong>' +
escapeHtml(item.title) + '</strong><small>' + escapeHtml( escapeHtml(item.title) + '</strong><small>' + escapeHtml(
(item.kind === 'pull' ? 'Pull request' : 'Issue') + ' · ' + item.repository + ' #' + item.number + (item.kind === 'pull' ? 'Pull request' : 'Issue') + ' · ' + item.repository + ' #' + item.number +
' · ' + item.state + ' · ' + formatTime(item.updated_at)) + ' · ' + item.state + ' · ' + formatTime(item.updated_at)) +

View File

@ -119,10 +119,15 @@
retry.hidden = older.hidden = true; retry.hidden = older.hidden = true;
return; return;
} }
const isNew = comment => Boolean(conversation.reviewedAt && comment.created_at &&
new Date(comment.created_at).getTime() > new Date(conversation.reviewedAt).getTime());
const newCount = (conversation.comments || []).filter(isNew).length;
comments.innerHTML = (conversation.comments || []).map(comment => comments.innerHTML = (conversation.comments || []).map(comment =>
'<article class="search-preview-comment" data-comment-id="' + Number(comment.id || 0) + '">' + '<article class="search-preview-comment' + (isNew(comment) ? ' new-since-review' : '') +
'" data-comment-id="' + Number(comment.id || 0) + '">' +
'<div class="small">' + escapeHtml(comment.author || 'Unknown author') + '<div class="small">' + escapeHtml(comment.author || 'Unknown author') +
(comment.created_at ? ' · ' + escapeHtml(formatTime(comment.created_at)) : '') + '</div>' + (comment.created_at ? ' · ' + escapeHtml(formatTime(comment.created_at)) : '') +
(isNew(comment) ? ' · <strong>New since last review</strong>' : '') + '</div>' +
'<div class="markdown-content">' + renderMarkdown(comment.body || '') + '</div></article>' '<div class="markdown-content">' + renderMarkdown(comment.body || '') + '</div></article>'
).join(''); ).join('');
retry.hidden = conversation.status !== 'error'; retry.hidden = conversation.status !== 'error';
@ -134,7 +139,8 @@
'Conversation unavailable. Preview and planning actions still work.'; 'Conversation unavailable. Preview and planning actions still work.';
else if (!conversation.comments?.length) status.textContent = 'No conversation yet.'; else if (!conversation.comments?.length) status.textContent = 'No conversation yet.';
else status.textContent = conversation.comments.length + else status.textContent = conversation.comments.length +
(conversation.comments.length === 1 ? ' message.' : ' messages.'); (conversation.comments.length === 1 ? ' message' : ' messages') +
(newCount ? ' · ' + newCount + ' new since last review' : '') + '.';
}; };
root.renderSearchPreviewReply = (state, detail, preview, document) => { root.renderSearchPreviewReply = (state, detail, preview, document) => {
const section = document.querySelector('.search-preview-reply'); const section = document.querySelector('.search-preview-reply');
@ -176,12 +182,21 @@
let replyRequest = null; let replyRequest = null;
let watchRequest = null; let watchRequest = null;
let conversation = null; let conversation = null;
let openedRevision = null;
function sameItem(left, right) { function sameItem(left, right) {
return left && right && left.kind === right.kind && left.repository === right.repository && return left && right && left.kind === right.kind && left.repository === right.repository &&
Number(left.number) === Number(right.number); Number(left.number) === Number(right.number);
} }
async function notifyOpened(item, requestGeneration) {
const revision = [item?.kind, item?.repository, item?.number, item?.updated_at].join(':');
if (requestGeneration !== generation || openedRevision === revision) return false;
await onOpened?.({...item});
if (requestGeneration === generation) openedRevision = revision;
return true;
}
function replyKey(item, suffix) { function replyKey(item, suffix) {
return 'stackchain.search-reply.' + [item?.kind, item?.repository, item?.number] return 'stackchain.search-reply.' + [item?.kind, item?.repository, item?.number]
.map(value => encodeURIComponent(String(value || ''))).join('.') + '.' + suffix; .map(value => encodeURIComponent(String(value || ''))).join('.') + '.' + suffix;
@ -246,7 +261,10 @@
if (typeof fetchConversation !== 'function') return Promise.resolve(null); if (typeof fetchConversation !== 'function') return Promise.resolve(null);
const previousComments = page && Array.isArray(conversation?.comments) const previousComments = page && Array.isArray(conversation?.comments)
? conversation.comments : []; ? conversation.comments : [];
conversation = { status:'loading', comments:previousComments, olderPage:page ?? null }; conversation = {
status:'loading', comments:previousComments, olderPage:page ?? null,
reviewedAt:detail?.following === true ? detail.reviewed_at : null,
};
publish({ status:'ready', item:current, detail, conversation }); publish({ status:'ready', item:current, detail, conversation });
return fetchConversation(detail, page).then(result => { return fetchConversation(detail, page).then(result => {
if (requestGeneration !== generation) return result; if (requestGeneration !== generation) return result;
@ -258,6 +276,7 @@
status:'ready', status:'ready',
comments, comments,
olderPage:result?.older_page ?? null, olderPage:result?.older_page ?? null,
reviewedAt:detail?.following === true ? detail.reviewed_at : null,
}; };
publish({ status:'ready', item:current, detail:current, conversation }); publish({ status:'ready', item:current, detail:current, conversation });
return result; return result;
@ -266,6 +285,7 @@
conversation = { conversation = {
status:'error', comments:previousComments, status:'error', comments:previousComments,
olderPage:page ?? conversation?.olderPage ?? null, error, olderPage:page ?? conversation?.olderPage ?? null, error,
reviewedAt:detail?.following === true ? detail.reviewed_at : null,
}; };
publish({ status:'ready', item:current, detail:current, conversation }); publish({ status:'ready', item:current, detail:current, conversation });
} }
@ -283,16 +303,23 @@
const requestGeneration = generation; const requestGeneration = generation;
current = { ...item }; current = { ...item };
conversation = null; conversation = null;
openedRevision = null;
publish({ status: 'loading', item: current }); publish({ status: 'loading', item: current });
return fetchJson(current).then(async detail => { return fetchJson(current).then(async detail => {
if (requestGeneration === generation) { if (requestGeneration === generation) {
current = { ...current, ...detail }; current = { ...current, ...detail };
if (typeof fetchConversation === 'function') { if (typeof fetchConversation === 'function') {
loadConversation(current, requestGeneration); const context = loadConversation(current, requestGeneration);
if (current.following === true) {
await context;
if (conversation?.status === 'ready') await notifyOpened(current, requestGeneration);
} else {
await notifyOpened(current, requestGeneration);
}
} else { } else {
publish({ status: 'ready', item: current, detail }); publish({ status: 'ready', item: current, detail });
await notifyOpened(current, requestGeneration);
} }
await onOpened?.({...current});
} }
return detail; return detail;
}).catch(error => { }).catch(error => {
@ -340,9 +367,14 @@
})().finally(() => { moveRequest = null; }); })().finally(() => { moveRequest = null; });
return moveRequest; return moveRequest;
}, },
retryConversation() { async retryConversation() {
if (!current) return Promise.resolve(null); if (!current) return null;
return loadConversation(current, generation); const requestGeneration = generation;
const result = await loadConversation(current, requestGeneration);
if (current?.following === true && conversation?.status === 'ready') {
await notifyOpened(current, requestGeneration);
}
return result;
}, },
loadOlderConversation() { loadOlderConversation() {
if (!current || !conversation?.olderPage) return Promise.resolve(null); if (!current || !conversation?.olderPage) return Promise.resolve(null);

View File

@ -112,6 +112,16 @@ class FollowingStore:
if not isinstance(kept_updated_at, str) or not kept_updated_at or len(kept_updated_at) > 64: if not isinstance(kept_updated_at, str) or not kept_updated_at or len(kept_updated_at) > 64:
raise ValueError("kept update is invalid") raise ValueError("kept update is invalid")
item["kept_updated_at"] = kept_updated_at item["kept_updated_at"] = kept_updated_at
reviewed_title = raw.get("reviewed_title")
if reviewed_title is not None:
if not isinstance(reviewed_title, str) or not reviewed_title.strip() or len(reviewed_title.strip()) > 300:
raise ValueError("reviewed title is invalid")
item["reviewed_title"] = reviewed_title.strip()
reviewed_state = raw.get("reviewed_state")
if reviewed_state is not None:
if reviewed_state not in _STATES:
raise ValueError("reviewed state is invalid")
item["reviewed_state"] = reviewed_state
return item return item
@classmethod @classmethod
@ -123,8 +133,19 @@ class FollowingStore:
unseen = (stored["updated_at"] != stored["last_seen_updated_at"] or unseen = (stored["updated_at"] != stored["last_seen_updated_at"] or
stored.get("kept_updated_at") == stored["updated_at"]) stored.get("kept_updated_at") == stored["updated_at"])
item = {key: value for key, value in stored.items() item = {key: value for key, value in stored.items()
if key not in {"last_seen_updated_at", "kept_updated_at"}} if key not in {"last_seen_updated_at", "kept_updated_at",
"reviewed_title", "reviewed_state"}}
item["has_unseen_change"] = unseen item["has_unseen_change"] = unseen
if unseen:
item["reviewed_at"] = stored["last_seen_updated_at"]
if stored.get("reviewed_state") == "open" and stored["state"] == "closed":
item["change_summary"] = "Closed since last review"
elif stored.get("reviewed_state") == "closed" and stored["state"] == "open":
item["change_summary"] = "Reopened since last review"
elif stored.get("reviewed_title") not in {None, stored["title"]}:
item["change_summary"] = "Title changed since last review"
else:
item["change_summary"] = "New activity"
(changed if unseen else unchanged).append(item) (changed if unseen else unchanged).append(item)
changed.sort(key=lambda item: item["updated_at"], reverse=True) changed.sort(key=lambda item: item["updated_at"], reverse=True)
return {"revision": snapshot["revision"], "items": changed + unchanged} return {"revision": snapshot["revision"], "items": changed + unchanged}
@ -194,8 +215,9 @@ class FollowingStore:
items.insert(0, item) items.insert(0, item)
else: else:
item["last_seen_updated_at"] = items[index]["last_seen_updated_at"] item["last_seen_updated_at"] = items[index]["last_seen_updated_at"]
if "kept_updated_at" in items[index]: for key in ("kept_updated_at", "reviewed_title", "reviewed_state"):
item["kept_updated_at"] = items[index]["kept_updated_at"] if key in items[index]:
item[key] = items[index][key]
if items[index] == item: if items[index] == item:
return self._present({"revision": current["revision"], "items": items}) return self._present({"revision": current["revision"], "items": items})
items.pop(index) items.pop(index)
@ -229,8 +251,12 @@ class FollowingStore:
if update is None: if update is None:
continue continue
update["last_seen_updated_at"] = item["last_seen_updated_at"] update["last_seen_updated_at"] = item["last_seen_updated_at"]
if "kept_updated_at" in item: for key in ("kept_updated_at", "reviewed_title", "reviewed_state"):
update["kept_updated_at"] = item["kept_updated_at"] if key in item:
update[key] = item[key]
if update["updated_at"] != item["updated_at"]:
update.setdefault("reviewed_title", item["title"])
update.setdefault("reviewed_state", item["state"])
if update != item: if update != item:
items[index] = update items[index] = update
changed = True changed = True
@ -272,6 +298,8 @@ class FollowingStore:
item.get("kept_updated_at") == updated_at): item.get("kept_updated_at") == updated_at):
item["last_seen_updated_at"] = updated_at item["last_seen_updated_at"] = updated_at
item.pop("kept_updated_at", None) item.pop("kept_updated_at", None)
item.pop("reviewed_title", None)
item.pop("reviewed_state", None)
revision += 1 revision += 1
connection.execute( connection.execute(
"UPDATE following_issues SET revision = ?, items = ? WHERE login = ?", "UPDATE following_issues SET revision = ?, items = ? WHERE login = ?",

View File

@ -41,7 +41,8 @@ def test_following_queue_is_phone_usable_at_narrow_viewport(viewport):
json:async () => ({revision:1,items:[{ json:async () => ({revision:1,items:[{
repository:'stackchain/api', kind:'pull', number:42, repository:'stackchain/api', kind:'pull', number:42,
title:'Changed pull request with a long mobile title', state:'open', title:'Changed pull request with a long mobile title', state:'open',
updated_at:'2026-08-23T05:00:00Z', has_unseen_change:true updated_at:'2026-08-23T05:00:00Z', has_unseen_change:true,
change_summary:'Closed since last review'
}]}) }]})
}); });
globalThis.followingReleaseQueue = attachFollowing(() => {}); globalThis.followingReleaseQueue = attachFollowing(() => {});
@ -50,7 +51,7 @@ def test_following_queue_is_phone_usable_at_narrow_viewport(viewport):
expect(page.locator("#following-sheet")).to_be_visible() expect(page.locator("#following-sheet")).to_be_visible()
expect(page.locator(".following-card")).to_be_visible() expect(page.locator(".following-card")).to_be_visible()
expect(page.locator(".following-card")).to_contain_text("New activity") expect(page.locator(".following-card")).to_contain_text("Closed since last review")
expect(page.locator(".following-card")).to_contain_text("Pull request") expect(page.locator(".following-card")).to_contain_text("Pull request")
expect(page.locator("#review-following")).to_have_text("Review new activity") expect(page.locator("#review-following")).to_have_text("Review new activity")
assert page.locator("#review-following").bounding_box()["height"] >= 44 assert page.locator("#review-following").bounding_box()["height"] >= 44

View File

@ -113,6 +113,69 @@ def test_following_preview_wires_keep_for_later_action_and_retryable_failure():
assert "keepForLater:feature.keepForLater" in following assert "keepForLater:feature.keepForLater" in following
def test_following_conversation_marks_messages_newer_than_last_review():
preview_module = ROOT / "frontend" / "search-preview.js"
script = f"""
require({json.dumps(str(preview_module))});
const nodes={{
'#search-preview-comments':{{innerHTML:'',textContent:''}},
'#search-preview-conversation-status':{{textContent:''}},
'#retry-search-preview-conversation':{{hidden:true}},
'#load-older-search-preview-comments':{{hidden:true,disabled:false}},
}};
const document={{querySelector:selector=>nodes[selector]}};
global.renderSearchPreviewConversation({{
status:'ready', reviewedAt:'2026-08-23T03:00:00Z', comments:[
{{id:1,author:'Timmy',created_at:'2026-08-23T02:00:00Z',body:'Earlier'}},
{{id:2,author:'Alex',created_at:'2026-08-23T04:00:00Z',body:'New decision'}},
]
}},document,String,String,String);
process.stdout.write(JSON.stringify({{html:nodes['#search-preview-comments'].innerHTML,status:nodes['#search-preview-conversation-status'].textContent}}));
"""
result = json.loads(subprocess.run(
["node", "-e", script], text=True, capture_output=True, check=True
).stdout)
assert result["html"].count("new-since-review") == 1
assert "New since last review" in result["html"]
assert result["status"] == "2 messages · 1 new since last review."
def test_following_revision_is_acknowledged_only_after_conversation_context_loads():
preview_module = ROOT / "frontend" / "search-preview.js"
script = f"""
const createSearchPreview=require({json.dumps(str(preview_module))});
let unavailable=true;
const opened=[];
const preview=createSearchPreview({{
fetchJson:async item=>({{...item,title:'Changed issue'}}),
fetchConversation:async()=>{{if(unavailable)throw new Error('offline');return {{comments:[]}};}},
onOpened:async item=>opened.push(item.updated_at),
onState:()=>{{}},
}});
(async()=>{{
await preview.open({{
repository:'stackchain/api',kind:'issue',number:42,following:true,
reviewed_at:'2026-08-23T03:00:00Z',updated_at:'2026-08-23T04:00:00Z'
}});
const afterFailure=[...opened];
unavailable=false;
await preview.retryConversation();
process.stdout.write(JSON.stringify({{afterFailure,opened}}));
}})().catch(error=>{{console.error(error);process.exit(1);}});
"""
result = json.loads(subprocess.run(
["node", "-e", script], text=True, capture_output=True, check=True
).stdout)
assert result == {
"afterFailure": [],
"opened": ["2026-08-23T04:00:00Z"],
}
def test_following_typed_identity_prevents_issue_pull_collisions(): def test_following_typed_identity_prevents_issue_pull_collisions():
script = f""" script = f"""
const createFollowing = require({json.dumps(str(MODULE))}); const createFollowing = require({json.dumps(str(MODULE))});

View File

@ -51,6 +51,34 @@ def test_refresh_marks_changed_items_unseen_and_sorts_them_first(tmp_path):
assert snapshot["items"][0]["state"] == "closed" assert snapshot["items"][0]["state"] == "closed"
def test_refresh_explains_change_from_last_reviewed_snapshot(tmp_path):
store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=KEY)
store.set_watching("timmy", ITEM, True)
closed = store.refresh("timmy", [{
**ITEM,
"title": "Make mobile review trustworthy",
"state": "closed",
"updated_at": "2026-08-23T04:00:00Z",
}])
assert closed["items"][0]["reviewed_at"] == ITEM["updated_at"]
assert closed["items"][0]["change_summary"] == "Closed since last review"
store.acknowledge(
"timmy", ITEM["repository"], ITEM["number"], "2026-08-23T04:00:00Z"
)
title_change = store.refresh("timmy", [{
**ITEM,
"title": "Make mobile review understandable",
"state": "closed",
"updated_at": "2026-08-23T05:00:00Z",
}])
assert title_change["items"][0]["reviewed_at"] == "2026-08-23T04:00:00Z"
assert title_change["items"][0]["change_summary"] == "Title changed since last review"
def test_acknowledgement_is_revision_conditional_and_later_change_is_unseen(tmp_path): def test_acknowledgement_is_revision_conditional_and_later_change_is_unseen(tmp_path):
store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=KEY) store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=KEY)
store.set_watching("timmy", ITEM, True) store.set_watching("timmy", ITEM, True)