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
9 changes: 9 additions & 0 deletions foc-board-rules/field-completeness.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,12 @@ If no reasonable inference can be made, **leave Cycle Theme blank** β€” do not i
**Scope:** Only applies when the item's Cycle is a *past* iteration. An item with no Cycle at all is [R-FC-012](#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle)'s concern, not this one's. An item whose Cycle is a *future* iteration (deliberately planned ahead) is left alone.
**Skip if a human has since moved it back:** If this rule (or any mutation this tool made under this rule's id) previously moved the item's Cycle *away from* the past-cycle value it currently holds, do not re-apply β€” flag for human review instead. A human moving an item back to a past cycle after this tool moved it forward is a deliberate signal (e.g. correcting a mistaken auto-move, or intentionally leaving it attributed to the cycle where the work actually happened) that automation shouldn't fight. See [`foc-mechanical-rules`'s "Mutation log" section](../foc-mechanical-rules/README.md#mutation-log) for how this is tracked and its limits (only reversions this tool's own history witnessed are caught).
**Why:** An item still open once its cycle has ended almost always means the cycle ended before the work did β€” the Cycle value is now stale and understates what's actually in flight this cycle. Leaving it in the old iteration hides the work from current cycle planning and reporting.

## R-FC-014: Recently-completed items without a Cycle get the current cycle

**Enforced mechanically, hourly:** this rule runs automatically via [`foc-mechanical-rules`](../foc-mechanical-rules/) (see [its implementation](../foc-mechanical-rules/foc_mechanical_rules/rules/cycle.py)), scheduled by [`.github/workflows/foc-board-mechanical-rules.yml`](../.github/workflows/foc-board-mechanical-rules.yml). The prose below stays canonical for *what* and *why*; the linked module is canonical for exactly how it's evaluated.

**When:** Any **issue or PR** on the board in "πŸŽ‰ Done" has no Cycle set and was updated within the last 3 days (`updated:>@today-3d`).
**Action:** Set the Cycle to the current active cycle (the iteration whose date range contains today).
**Relationship to [R-FC-012](#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle):** Same "no cycle -> assign current cycle" logic and the same 3-day window, but R-FC-012 explicitly excludes Done items β€” this rule is that gap's Done-side counterpart, catching items that moved straight to Done without ever getting a Cycle (e.g. via R-PR-008/R-PR-009's merged/closed-to-Done moves).
**Why:** A completed item with no Cycle is invisible in cycle-based reporting (velocity, burndown, "what shipped this cycle") even though the work is done and attributable. Left uncorrected, this silently undercounts every cycle's completed work.
2 changes: 2 additions & 0 deletions foc-mechanical-rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Neither `mutation_log.py` nor any rule module knows or cares how the log survive
| [R-PR-001](../foc-board-rules/pr-hygiene.md#r-pr-001-unassigned-prs-should-be-assigned-to-their-author) | assignee | Unassigned open PRs are assigned to their author (skipping bots, with a merged-release-PR carve-out, and skipping PRs where a human explicitly removed the assignee) |
| [R-FC-012](../foc-board-rules/field-completeness.md#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle) | cycle | Issues/PRs updated in the last 3 days with no Cycle get the current cycle, unless this tool previously set that item's Cycle to the current one and a human has since cleared it |
| [R-FC-013](../foc-board-rules/field-completeness.md#r-fc-013-open-items-in-a-past-cycle-should-move-to-the-current-cycle) | cycle | Open issues/PRs whose Cycle is a past iteration move to the current cycle, unless this tool previously moved that item off the same past cycle and a human has since moved it back |
| [R-FC-014](../foc-board-rules/field-completeness.md#r-fc-014-recently-completed-items-without-a-cycle-get-the-current-cycle) | cycle | Issues/PRs in Done, updated in the last 3 days, with no Cycle get the current cycle (same guard against re-adding a human-cleared cycle as R-FC-012) |
| [R-PR-010](../foc-board-rules/pr-hygiene.md#r-pr-010-triage-prs-should-be-routed-to-the-correct-status) | status | PRs in Triage are routed to In Progress / Todo / Approved by reviewer / Awaiting review per the [PR status determination table](../foc-board-rules/pr-status-table.md), unless GitHub's real Status-field history shows a human explicitly moved the PR back to Triage |

## API call pattern per rule
Expand All @@ -49,6 +50,7 @@ Board size (~180 open items and growing) makes it easy for a rule to accidentall
| R-PR-001 (assignee) | 1 paginated board query | β€” | 1 REST `GET` (PR metadata) per candidate; **+1 REST `GET`** (issue events, paginated) unless skipped as a bot author | 1 REST `POST` per applied item (not batched β€” see gap below) |
| R-FC-012 (cycle) | 1 paginated board query | 1 GraphQL query (iterations) | none | Batched: all applied items in the run share 1 GraphQL mutation per 25 items (`_CycleFieldRule.mutate_pending`) |
| R-FC-013 (cycle) | 1 paginated board query (Cycle field included, so no separate read is needed to know an item's current cycle) | 1 GraphQL query (iterations, shared helper with R-FC-012) | none | Batched, same mechanism as R-FC-012 (via the shared `_CycleFieldRule.mutate_pending`) |
| R-FC-014 (cycle) | 1 paginated board query (Done items only, same 3-day window as R-FC-012) | 1 GraphQL query (iterations, shared helper with R-FC-012) | none | Batched, same mechanism as R-FC-012 (`DoneCycleRule` subclasses `CycleRule`, overriding only the `_STATUS_FILTER` class attribute) |
| R-PR-010 (status) | 1 paginated board query (Triage items only) | β€” | 1 GraphQL query per candidate (`get_pr_review_context`: draft/author/commits/reviews/reviewRequests/comments/status history in one round trip); **+1 REST `GET`** (collaborator permission) per unique reviewer login with a qualifying review, cached per (owner, repo, login) for the run | Batched: all applied items grouped by target status, 1 GraphQL mutation per 25 items per group (same `mutate_pending` pattern as `_CycleFieldRule`) |

**Known gap, not yet worth fixing:** R-PR-001's 1-2 REST reads per candidate PR are real per-item calls with no batched equivalent used today, unlike the two cycle rules. At current volume (dozens of candidates per hourly run, most REST GETs) it's well within GitHub's rate limits and not worth the complexity, but if candidate volume grows a lot, `github_projects_client`'s `nodes(ids: [ID!]!)` batching pattern (used by `set_field_value_bulk`'s old-value fetch) generalizes: a GraphQL `nodes()` query keyed by PR node IDs could fetch author + merge state for many PRs in one call, cutting the metadata `GET` to near-zero; issue-events (used only to detect a human `unassigned` event) would need a similar `timelineItems` batch to fully close the gap.
Expand Down
10 changes: 8 additions & 2 deletions foc-mechanical-rules/foc_mechanical_rules/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,15 @@

from .rule import Rule
from .rules.assignee import AssigneeRule
from .rules.cycle import CycleRule, PastCycleRule
from .rules.cycle import CycleRule, DoneCycleRule, PastCycleRule
from .rules.pr_status import PRStatusRule


def default_rules() -> List[Rule]:
return [AssigneeRule(), CycleRule(), PastCycleRule(), PRStatusRule()]
return [
AssigneeRule(),
CycleRule(),
PastCycleRule(),
DoneCycleRule(),
PRStatusRule(),
]
32 changes: 30 additions & 2 deletions foc-mechanical-rules/foc_mechanical_rules/rules/cycle.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
"""R-FC-012 / R-FC-013: keep an item's Cycle pointed at the current iteration.
"""R-FC-012 / R-FC-013 / R-FC-014: keep an item's Cycle pointed at the current iteration.

Canonical English rules:
- foc-board-rules/field-completeness.md#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle
- foc-board-rules/field-completeness.md#r-fc-013-open-items-in-a-past-cycle-should-move-to-the-current-cycle
- foc-board-rules/field-completeness.md#r-fc-014-recently-completed-items-without-a-cycle-get-the-current-cycle

This module is that rule's canonical implementation β€” see rules/assignee.py's
docstring for why the markdown and this module link back to each other.
Expand Down Expand Up @@ -179,11 +180,20 @@ def mutate_pending(


class CycleRule(_CycleFieldRule):
"""Recently-active items with no Cycle get the current cycle.

``_STATUS_FILTER`` is the only thing ``DoneCycleRule`` (R-FC-014)
overrides to change its selection query -- both rules share the same
"no cycle in the last 3 days" query shape, just scoped to Done vs.
not-Done items.
"""

id = "R-FC-012"
doc_url = (
"https://github.com/FilOzone/tpm-utils/blob/master/foc-board-rules/"
"field-completeness.md#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle"
)
_STATUS_FILTER = '-status:"πŸŽ‰ Done"'

def __init__(self, org: str = FILOZ_ORG, project_number: int = PROJECT_NUMBER):
self.org = org
Expand All @@ -207,7 +217,7 @@ def select(self, session: requests.Session) -> List[Dict[str, Any]]:
session,
org=self.org,
project_number=self.project_number,
query='-status:"πŸŽ‰ Done" no:cycle updated:>@today-3d',
query=f"{self._STATUS_FILTER} no:cycle updated:>@today-3d",
Comment thread
BigLep marked this conversation as resolved.
fields=["Repository", "Id", "Title", "url"],
cursor=cursor,
)
Expand Down Expand Up @@ -278,6 +288,24 @@ def apply_one(
)


class DoneCycleRule(CycleRule):
"""Same "no cycle -> assign current cycle" logic as R-FC-012, scoped to
items currently in Done instead of items that are still active.

Subclasses ``CycleRule`` and only overrides ``_STATUS_FILTER`` to
change its selection query (plus ``id``/``doc_url`` for identity) --
``select()``'s query shape, ``apply_one``, and the mutation-log guard
are otherwise identical between the two rules.
"""

id = "R-FC-014"
doc_url = (
"https://github.com/FilOzone/tpm-utils/blob/master/foc-board-rules/"
"field-completeness.md#r-fc-014-recently-completed-items-without-a-cycle-get-the-current-cycle"
)
_STATUS_FILTER = 'status:"πŸŽ‰ Done"'


class PastCycleRule(_CycleFieldRule):
id = "R-FC-013"
doc_url = (
Expand Down
2 changes: 1 addition & 1 deletion foc-mechanical-rules/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def test_no_rule_flag_runs_every_registered_rule(capsys):
_run_cli(["--dry-run", "--token", "x"], run_all_mock)

ran_ids = {rule.id for rule in run_all_mock.call_args.args[1]}
assert ran_ids == {"R-PR-001", "R-FC-012", "R-FC-013", "R-PR-010"}
assert ran_ids == {"R-PR-001", "R-FC-012", "R-FC-013", "R-FC-014", "R-PR-010"}


def test_rule_flag_filters_to_the_named_rule(capsys):
Expand Down
42 changes: 41 additions & 1 deletion foc-mechanical-rules/tests/test_cycle_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@

from foc_mechanical_rules.mutation_log import MutationLog, MutationRecord
from foc_mechanical_rules.rule import ActionResult, Rule
from foc_mechanical_rules.rules.cycle import CycleRule, get_current_cycle_title
from foc_mechanical_rules.rules.cycle import (
CycleRule,
DoneCycleRule,
get_current_cycle_title,
)

ITEM = {
"Repository": "FilOzone/dealbot",
Expand Down Expand Up @@ -204,3 +208,39 @@ def test_mutate_pending_reports_per_item_failure(mock_bulk):

def test_cycle_rule_is_a_rule():
assert isinstance(CycleRule(), Rule)


@patch("foc_mechanical_rules.rules.cycle.list_items")
def test_done_cycle_rule_queries_done_items_with_the_same_window_as_cycle_rule(
mock_list_items,
):
mock_list_items.return_value = {"items": [], "has_more": False, "next_cursor": None}

DoneCycleRule().select(MagicMock())

query = mock_list_items.call_args.kwargs["query"]
assert 'status:"πŸŽ‰ Done"' in query
assert '-status:"πŸŽ‰ Done"' not in query
assert "no:cycle" in query
assert "updated:>@today-3d" in query


@patch(
"foc_mechanical_rules.rules.cycle.get_current_cycle_title", return_value="202608-2"
)
def test_done_cycle_rule_reuses_cycle_rules_apply_one(mock_get_cycle):
# DoneCycleRule only overrides the status filter used by select() --
# apply_one's behavior (queue a pending mutation for a candidate with no
# prior history) should match CycleRule's exactly.
rule = DoneCycleRule()
result = rule.apply_one(
MagicMock(), ITEM, dry_run=False, mutation_log=MutationLog()
)

assert result.status == "pending"
assert result.new_value == "202608-2"
assert rule.id == "R-FC-014"


def test_done_cycle_rule_is_a_rule():
assert isinstance(DoneCycleRule(), Rule)
Loading