Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/agent_comms/todos.py
Original file line number Diff line number Diff line change
Expand Up @@ -2577,7 +2577,12 @@ def claimed(self, claim: OrderClaim) -> ClaimedProblemOrder:
raise TodoError("Only a pending order can be claimed by its holder.")

def review_verdicts(self) -> tuple[type[HeadVerdict], ...]:
return (ReviewedHeadVerdict, RejectedHeadVerdict)
"""Any order that holds a head takes a decision on it, or a reviewer's mark first."""
return (ReviewedHeadVerdict, RejectedHeadVerdict, UnderReviewHeadVerdict)

def under_review(self, record: OrderHeadReviewRecord) -> CheckedProblemOrder:
"""Only a claim owes a delivery verdict; elsewhere the head record is the whole mark."""
return self

def sent_back(self, reason: StrikeReason) -> CheckedProblemOrder:
return self
Expand Down Expand Up @@ -3189,8 +3194,8 @@ def closed_by_merge(
def owed_claims(self) -> tuple[OrderClaim, ...]:
return (self.claim,)

def review_verdicts(self) -> tuple[type[HeadVerdict], ...]:
return (*super().review_verdicts(), UnderReviewHeadVerdict)
def under_review(self, record: OrderHeadReviewRecord) -> ReviewingProblemOrder:
return self.copy_as(ReviewingProblemOrder, review=record)


@dataclass(frozen=True, kw_only=True)
Expand Down Expand Up @@ -4398,8 +4403,7 @@ def description(self, reviewer: str, subject: str) -> str:
return f"Under review by {reviewer} on {subject}"

def judge_order(self, current: Todo, record: OrderHeadReviewRecord) -> Todo:
return replace(current, order=current.order.checked().copy_as(
ReviewingProblemOrder, review=record))
return replace(current, order=current.order.checked().under_review(record))


class HeadOrderLink(HeadReviewRelation, declared_name=None):
Expand Down
8 changes: 4 additions & 4 deletions tests/test_board_verbs.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def test_held_board_commands_take_the_same_verdicts_as_the_order(board_review, c
rules = LinkedUnreviewedHead.scan_reviews(comms.goals, "lead", ProgressRead(()))
rule, = rules
assert rule.todo == todo
assert (UnderReviewHeadVerdict in assert_review_preconditions(rule, comms, request)) == claim_it
assert UnderReviewHeadVerdict in assert_review_preconditions(rule, comms, request)
frame = capture_frame(comms, "lead", ProgressRead(()))
# The timer's own condition remains real: age the observed review timer in this read.
timed = replace(todo, review_at=RecordedEventTime(time.time() - 120))
Expand Down Expand Up @@ -166,15 +166,15 @@ def test_shared_head_keeps_each_order_on_its_own_board_and_timer_line(board_revi
assert {rule.todo.id for rule in rules} == {first.id, second.id}
for rule in rules:
verdicts = assert_review_preconditions(rule, comms, request)
assert (UnderReviewHeadVerdict in verdicts) == (rule.todo.id == second.id)
assert UnderReviewHeadVerdict in verdicts
frame = capture_frame(comms, "lead", ProgressRead(()))
timed = replace(first, review_at=RecordedEventTime(time.time() - 120))
frame = replace(frame, live=(LiveHold(timed, comms.registry.require("worker")),))
timers = ReviewTimerUp.scan_observed(comms, "lead", (), frame)
timer, = timers
assert timer.todo == timed
assert timer.problem == first.id
assert UnderReviewHeadVerdict not in assert_review_preconditions(timer, comms, request)
assert UnderReviewHeadVerdict in assert_review_preconditions(timer, comms, request)


def test_orderless_board_commands_do_not_invent_a_problem(board_review):
Expand Down Expand Up @@ -203,7 +203,7 @@ def test_claim_link_survives_sendbacks_and_explicit_release_without_editing_goal
assert released.order.holds_head(request)
assert comms.goals.todos.resolve_order(released.creator_identity, previous.owner, released.id) == released
rule, = LinkedUnreviewedHead.scan_reviews(comms.goals, "lead", ProgressRead(()))
assert UnderReviewHeadVerdict not in assert_review_preconditions(rule, comms, request)
assert UnderReviewHeadVerdict in assert_review_preconditions(rule, comms, request)
# A former holder can already be doing different work. A head review must not edit that goal.
other = comms.goals.todos.get("P1")
ReleaseTodo(other.revision, other.creator_identity, other.held(), StrikeReason(
Expand Down
19 changes: 19 additions & 0 deletions tests/test_head_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,25 @@ def test_linked_head_rejection_strikes_revises_and_notifies_its_holder(head_bus)
assert StoredHeadReview.head_reviews(comms.goals.todos, request)[0].order.problem == todo.id


def test_a_reviewing_mark_on_an_unclaimed_linked_order_is_recorded(head_bus):
"""Every order that takes ok or back on its linked head also takes the reviewing mark."""
comms, _, request = head_bus
todo = comms.goals.todos.get("P2")
held = todo.held()
RecordPullRequest(todo.revision, held.owner_identity,
PullRequestLink(request.repo, request.pr), held.generation).record(comms.goals, todo.id)
todo = comms.goals.todos.get(todo.id)
scope = OrderHeadReview.for_request(
(todo,), request, lambda linked: UnquotedHeadReview.of(linked, ()))
invoke(comms, request, UnderReviewHeadVerdict(), scope)
changed = comms.goals.todos.get(todo.id)
assert changed.order == todo.order
assert changed.revision == todo.revision + 1
review, = StoredHeadReview.head_reviews(comms.goals.todos, request)
assert type(review.verdict) is UnderReviewHeadVerdict
assert review.order.problem == todo.id


def test_acceptance_over_the_growth_limit_needs_a_reason_naming_each_growing_file(
head_bus, reviewed_source):
comms, github, _ = head_bus
Expand Down
Loading