Activity needs review; this is not a decision to reopen. Contributors may lack reopening permission.
' + historical = row.get('historical_follow_up_at') + if historical: + details += 'Last external follow-up:
' + details += 'Older than 30 days; kept here as history.
' + else: + details += 'Activity needs review; this is not a decision to reopen. Contributors may lack reopening permission.
' if row.get('waiting_since'): details += 'Waiting since:
' if stale: diff --git a/ears/collect.py b/ears/collect.py index cc84248..6254d9a 100644 --- a/ears/collect.py +++ b/ears/collect.py @@ -4,11 +4,12 @@ import json import re import subprocess -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from pathlib import Path REPO = re.compile(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+\Z") BROADCAST = {"Announcements", "Show and tell"} +FOLLOW_UP_WINDOW = timedelta(days=30) DISCUSSION_COMMENTS = """query($owner:String!, $name:String!, $number:Int!, $after:String) { repository(owner:$owner, name:$name) { discussion(number:$number) { comments(first:100, after:$after) { @@ -312,7 +313,7 @@ def validate_follow_up(row): raise ValueError("inconsistent follow-up response") -def conversation(api, repo, raw, kind, config, deliveries=None, pending=None): +def conversation(api, repo, raw, kind, config, deliveries=None, pending=None, observed_at=None): number = raw.get("number") if not isinstance(number, int) or isinstance(number, bool) or number < 1: raise ReadError("invalid conversation number") @@ -351,6 +352,7 @@ def conversation(api, repo, raw, kind, config, deliveries=None, pending=None): category = (raw.get("category") or {}).get("name") answered = raw.get("answer_chosen_at") is not None observation = None + historical_follow_up_at = None settled = raw.get("state") == "closed" or (kind == "discussion" and answered) if kind != "pr" and settled and category not in BROADCAST: # REST Discussions omit closed_at. Missing/unreadable GraphQL evidence @@ -372,6 +374,16 @@ def conversation(api, repo, raw, kind, config, deliveries=None, pending=None): observation = follow_up(raw, comments, kind, selves, complete, f"https://github.com/{repo}/{path}/{number}") awaiting, since = observation["review_needed"], observation["since"] + if awaiting is True: + # Only complete evidence reaches here. A fresh external reply renews + # attention even when the oldest pending comment is years old. + comment_events, _ = activity_events(comments) + latest = max(utc(t) for t, who in comment_events + if who.casefold() not in selves and utc(t) >= utc(since)) + if utc(observed_at or now()) - latest > FOLLOW_UP_WINDOW: + historical_follow_up_at = latest.isoformat() + awaiting, since = False, None + observation = {"review_needed": False, "since": None, "url": None} if awaiting is None: complete = False gaps.append("post-settlement activity or settlement time unknown") @@ -397,6 +409,7 @@ def conversation(api, repo, raw, kind, config, deliveries=None, pending=None): "gaps": gaps, "cached": False, "feedback_report": "" in (raw.get("body") or ""), **({"follow_up": observation} if observation is not None else {}), + **({"historical_follow_up_at": historical_follow_up_at} if historical_follow_up_at else {}), } @@ -463,7 +476,8 @@ def collect(api, repo_homes, config, pending=None): if item_kind == "issue" and is_human and not external and entry.get("state") != "closed": continue try: - row = conversation(api, repo, entry, item_kind, config, deliveries, pending) + row = conversation(api, repo, entry, item_kind, config, deliveries, pending, + observed_at=stamp) except ReadError as exc: receipt["gaps"].append(str(exc)) continue @@ -475,7 +489,8 @@ def collect(api, repo_homes, config, pending=None): and row["coverage"] == "complete": continue # Quiet closed issues add no queue value; keep unknowns visible. - if item_kind == "issue" and row.get("closed") and row["awaiting_response"] is False: + if item_kind == "issue" and row.get("closed") and row["awaiting_response"] is False \ + and not row.get("historical_follow_up_at"): continue source_rows.append(row) receipt["status"] = "partial" if receipt["gaps"] else "complete" @@ -516,7 +531,7 @@ def validate(snapshot): for row in rows: if not isinstance(row, dict): raise ValueError("invalid conversation") - if set(row) - {"closed", "follow_up", "created_at", "updated_at"} != {"id", "repo", "number", "kind", "url", "title", "author", "category", "answered", + if set(row) - {"closed", "follow_up", "created_at", "updated_at", "historical_follow_up_at"} != {"id", "repo", "number", "kind", "url", "title", "author", "category", "answered", "awaiting_response", "waiting_since", "review_requested", "coverage", "gaps", "cached", "feedback_report"}: raise ValueError("unknown conversation fields; raw content must not be published") if "closed" in row and type(row["closed"]) is not bool: @@ -534,6 +549,12 @@ def validate(snapshot): raise ValueError("invalid or duplicate conversation identity/link") ids.add(identity) validate_follow_up(row) + if "historical_follow_up_at" in row: + historical = utc(row["historical_follow_up_at"]) + if row.get("follow_up") != {"review_needed": False, "since": None, "url": None} \ + or row.get("coverage") != "complete" or row.get("waiting_since") is not None \ + or utc(snapshot["generated"]) - historical <= FOLLOW_UP_WINDOW: + raise ValueError("invalid historical follow-up evidence") if row.get("awaiting_response") is not None and type(row["awaiting_response"]) is not bool: raise ValueError("invalid awaiting-response state") if row.get("waiting_since"): diff --git a/ears/presentation.py b/ears/presentation.py index 19013b4..bc792bb 100644 --- a/ears/presentation.py +++ b/ears/presentation.py @@ -107,6 +107,8 @@ def progress(row, delivery, stale, plans): def waiting_label(row, observed): + if row.get('historical_follow_up_at'): + return 'Historical follow-up' if row.get('follow_up', {}).get('review_needed') is True: return 'Follow-up needs review' since = row.get('waiting_since') diff --git a/tests/board_browser.py b/tests/board_browser.py index 0849e3e..b659a30 100644 --- a/tests/board_browser.py +++ b/tests/board_browser.py @@ -9,7 +9,7 @@ from playwright.sync_api import sync_playwright from test_ears import BRAIN, CONFIG, STAMP, board, fixture from test_followthrough import delivery_snapshot -from test_closed_followups import candidate_snapshot +from test_closed_followups import candidate_snapshot, historical_snapshot def main(): @@ -36,6 +36,11 @@ def main(): followup['follow_up'].update(since=followup['waiting_since'], url=followup['url'] + '#discussioncomment-42') snapshot['conversations'].append(followup) + historical = historical_snapshot()['conversations'][0] + historical.update(number=7, id='example/lib/issues/7', + url='https://github.com/example/lib/issues/7', + title='Old follow-up on a closed issue') + snapshot['conversations'].append(historical) for name, value in board.render(snapshot, CONFIG, BRAIN).items(): (output / name).write_text(value) server = http.server.ThreadingHTTPServer( @@ -95,6 +100,13 @@ def main(): assert "community" in copied and "https://github.com/example/lib/issues/1" in copied page.locator('.board-nav-card[href="#activity"]').click() page.wait_for_function("document.getElementById('activity').closest('details.board-section').open") + historical_topic = page.locator('#activity .topic').filter(has_text='Old follow-up on a closed issue') + assert historical_topic.count() == 1 + assert page.locator('#attention .topic').filter(has_text='Old follow-up on a closed issue').count() == 0 + assert 'Historical follow-up' in historical_topic.locator('xpath=ancestor::tr').inner_text() + historical_topic.locator(':scope > summary').click() + assert 'Older than 30 days' in historical_topic.inner_text() + assert 'Activity needs review' not in historical_topic.inner_text() page.locator('#activity .topic summary').first.click() follow = page.get_by_role("button", name="Copy follow-through prompt").first follow.click() diff --git a/tests/test_closed_followups.py b/tests/test_closed_followups.py index 33555a8..c7b49e7 100644 --- a/tests/test_closed_followups.py +++ b/tests/test_closed_followups.py @@ -1,6 +1,8 @@ """New requests must be visible without requiring a contributor to reopen.""" import copy import json +from datetime import timedelta +from unittest.mock import patch import pytest @@ -12,6 +14,11 @@ LATER = '2026-10-03T00:00:00Z' +@pytest.fixture(autouse=True) +def fixed_collection_time(monkeypatch): + monkeypatch.setattr(collect, 'now', lambda: STAMP) + + def settled(**kwargs): return thread(state='closed', closed_at=CLOSED, answer_chosen_at=CLOSED, category={'name': 'Ideas & Proposals'}, **kwargs) @@ -106,11 +113,106 @@ def test_truncated_closed_issue_scan_retains_candidates_and_reports_gap(): def candidate_snapshot(): api = DiscussionAPI({(None, None): connection([node('new', stamp=LATER, databaseId=18676930)])}, raw=settled()) - s = collect.collect(api, [], CONFIG) - s['generated'] = STAMP + with patch('ears.collect.now', return_value=STAMP): + s = collect.collect(api, [], CONFIG) return s +def historical_snapshot(kind='issue', latest='2020-02-02T00:00:00Z', extra=()): + raw = thread(state='closed', created_at='2020-01-01T00:00:00Z', + closed_at='2020-02-01T00:00:00Z', updated_at=STAMP) + if kind != 'issue': + raw.update(answer_chosen_at=raw['closed_at'], category={'name': 'Help & Questions'}) + if kind == 'answered': + raw['state'] = 'open' + api = DiscussionAPI({(None, None): connection([ + node('old', stamp='2020-02-02T00:00:00Z'), + node('latest', stamp=latest), *extra])}, raw=raw) + else: + api = API({closed_listing(): [raw], comments(): [ + {'id': 41, 'user': user('Reporter'), 'created_at': '2020-02-02T00:00:00Z'}, + {'id': 42, 'user': user('Reporter'), 'created_at': latest}, *extra]}) + with patch('ears.collect.now', return_value=STAMP): + snapshot = collect.collect(api, ['example/lib'] if kind == 'issue' else [], CONFIG) + collect.validate(snapshot) + return snapshot + + +@pytest.mark.parametrize('kind', ['issue', 'discussion', 'answered']) +@pytest.mark.parametrize('seconds_old,historical', [(30 * 86400 + 1, True), (30 * 86400, False), (29 * 86400, False)]) +def test_cutoff_uses_latest_external_followup_not_oldest_pending_or_thread_update(kind, seconds_old, historical): + latest = (collect.utc(STAMP) - timedelta(seconds=seconds_old)).isoformat() + s = historical_snapshot(kind, latest) + row = s['conversations'][0] + assert row['awaiting_response'] is not historical + assert row['follow_up']['review_needed'] is not historical + if historical: + assert row['historical_follow_up_at'] == latest + assert row['waiting_since'] is None + assert row['follow_up'] == {'review_needed': False, 'since': None, 'url': None} + else: + assert 'historical_follow_up_at' not in row + assert row['waiting_since'] == '2020-02-02T00:00:00+00:00' + + +def test_historical_followup_stays_in_activity_and_out_of_feed_attention(): + s = historical_snapshot() + result = board.render(s, CONFIG, BRAIN, STAMP) + attention, activity = result['index.html'].split('id="activity"', 1) + assert '