sysadmin
ad59053cd7
The author allocator could select an ineligible issue for two independent reasons, both fixed here. Defect 1 — candidate truncation before ranking. The loader fetched the complete open inventory via api_get_all, then sliced it to `limit` items before constructing any WorkCandidate. Because every status:ready issue ties at priority 20 and the tie breaks on lowest number, the slice — not the ranking — decided the winner, so the same query returned different answers at different `limit` values. Candidate construction now consumes the full listing; `limit` bounds the reported skip list only, and any such truncation is reported explicitly rather than silently. Defect 2 — dependency state inferred from body substrings. Only "blocked on #" and "downstream of #" were recognized, so the repository's canonical `Depends: #N, #N` field never set dependency_unmet and dependency-blocked issues were emitted as eligible. The new allocator_dependencies module parses the canonical declaration into structured references and resolves each against live issue state: open means unmet, closed means met, and unavailable evidence fails closed. The complete open listing proves openness without extra calls; anything absent from it is confirmed by a cached targeted lookup instead of being assumed closed. The legacy "blocked on #" marker still parses. A failed listing now marks the inventory incomplete and the tool fails closed instead of ranking a partial set. Selection already advanced past a skipped candidate; with dependencies resolved correctly that fall-through now actually engages, and is covered by a regression. Tests: 35 new cases across parser, resolver, loader, and an MCP-level gitea_allocate_next_work regression, including limit-invariant selection over a 73-candidate inventory and a guard proving pre-ranking truncation would change the winner. No issue number is special-cased.
267 lines
9.9 KiB
Python
267 lines
9.9 KiB
Python
"""Dependency parsing/resolution and allocator completeness tests (#758).
|
|
|
|
Covers the two defects behind #758:
|
|
|
|
* Defect 1 — candidate truncation before ranking, which let a result-size
|
|
parameter change the winner.
|
|
* Defect 2 — dependency state inferred from body substrings, which emitted
|
|
canonical ``Depends:`` blocked issues as eligible.
|
|
|
|
No production behavior is special-cased for any issue number (#758 AC14), so
|
|
these tests use synthetic issue numbers throughout.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import tempfile
|
|
import unittest
|
|
|
|
import allocator_dependencies
|
|
from allocator_service import (
|
|
OUTCOME_PREVIEW,
|
|
SELECTION_POLICY,
|
|
WorkCandidate,
|
|
allocate_next_work,
|
|
classify_skip,
|
|
sort_candidates,
|
|
)
|
|
from control_plane_db import ControlPlaneDB
|
|
|
|
# The canonical linkage line this repository writes into issue bodies.
|
|
CANONICAL_BODY = (
|
|
"## Dependencies and linkage\n\n"
|
|
"* Parent: #900 · Depends: #901, #902 · Related: #903, #904\n"
|
|
)
|
|
|
|
|
|
class ParseDependencyRefsTest(unittest.TestCase):
|
|
def test_parses_canonical_depends_field(self) -> None:
|
|
self.assertEqual(
|
|
allocator_dependencies.parse_dependency_refs(CANONICAL_BODY),
|
|
(901, 902),
|
|
)
|
|
|
|
def test_stops_at_sibling_field_and_ignores_related(self) -> None:
|
|
""""Related:" refs must never be treated as dependencies."""
|
|
refs = allocator_dependencies.parse_dependency_refs(CANONICAL_BODY)
|
|
self.assertNotIn(903, refs)
|
|
self.assertNotIn(904, refs)
|
|
self.assertNotIn(900, refs) # Parent is not a dependency
|
|
|
|
def test_single_reference(self) -> None:
|
|
body = "* Parent: #10 · Depends: #11 · Related: #12"
|
|
self.assertEqual(allocator_dependencies.parse_dependency_refs(body), (11,))
|
|
|
|
def test_depends_on_spelling_and_and_separator(self) -> None:
|
|
body = "Depends on #21 and #22\n"
|
|
self.assertEqual(
|
|
allocator_dependencies.parse_dependency_refs(body), (21, 22)
|
|
)
|
|
|
|
def test_newline_terminates_declaration(self) -> None:
|
|
body = "Depends: #31, #32\nRelated: #33\n"
|
|
self.assertEqual(
|
|
allocator_dependencies.parse_dependency_refs(body), (31, 32)
|
|
)
|
|
|
|
def test_legacy_blocked_on_marker_still_recognized(self) -> None:
|
|
body = "This work is blocked on #41 until that lands.\n"
|
|
self.assertEqual(allocator_dependencies.parse_dependency_refs(body), (41,))
|
|
|
|
def test_dependencies_heading_alone_is_not_a_declaration(self) -> None:
|
|
""""Dependencies and linkage" must not parse as "Depends"."""
|
|
body = "## Dependencies and linkage\n\n* Related: #51\n"
|
|
self.assertEqual(allocator_dependencies.parse_dependency_refs(body), ())
|
|
|
|
def test_deduplicates_and_preserves_order(self) -> None:
|
|
body = "Depends: #61, #62, #61\n"
|
|
self.assertEqual(
|
|
allocator_dependencies.parse_dependency_refs(body), (61, 62)
|
|
)
|
|
|
|
def test_malformed_and_empty_inputs(self) -> None:
|
|
for body in ("", None, "Depends:", "Depends: none", "Depends: TBD\n"):
|
|
self.assertEqual(allocator_dependencies.parse_dependency_refs(body), ())
|
|
|
|
|
|
class ResolveDependencyStateTest(unittest.TestCase):
|
|
def test_open_dependency_is_unmet(self) -> None:
|
|
result = allocator_dependencies.resolve_dependency_state(
|
|
(901, 902), lambda n: "open", subject="issue#644"
|
|
)
|
|
self.assertTrue(result["dependency_unmet"])
|
|
self.assertEqual(result["unmet"], (901, 902))
|
|
self.assertIn("#901", result["reason"])
|
|
|
|
def test_closed_dependencies_are_met(self) -> None:
|
|
result = allocator_dependencies.resolve_dependency_state(
|
|
(901, 902), lambda n: "closed"
|
|
)
|
|
self.assertFalse(result["dependency_unmet"])
|
|
self.assertEqual(result["met"], (901, 902))
|
|
self.assertIsNone(result["reason"])
|
|
|
|
def test_mixed_open_and_closed_is_unmet(self) -> None:
|
|
states = {901: "closed", 902: "open"}
|
|
result = allocator_dependencies.resolve_dependency_state(
|
|
(901, 902), states.get
|
|
)
|
|
self.assertTrue(result["dependency_unmet"])
|
|
self.assertEqual(result["unmet"], (902,))
|
|
self.assertEqual(result["met"], (901,))
|
|
|
|
def test_unavailable_evidence_fails_closed(self) -> None:
|
|
"""AC7: unknown state must block, never pass."""
|
|
result = allocator_dependencies.resolve_dependency_state(
|
|
(901,), lambda n: None
|
|
)
|
|
self.assertTrue(result["dependency_unmet"])
|
|
self.assertEqual(result["unavailable"], (901,))
|
|
self.assertIn("fail closed", result["reason"])
|
|
|
|
def test_raising_lookup_fails_closed(self) -> None:
|
|
def boom(_n: int) -> str:
|
|
raise RuntimeError("lookup exploded")
|
|
|
|
result = allocator_dependencies.resolve_dependency_state((901,), boom)
|
|
self.assertTrue(result["dependency_unmet"])
|
|
self.assertEqual(result["unavailable"], (901,))
|
|
|
|
def test_no_refs_is_eligible(self) -> None:
|
|
result = allocator_dependencies.resolve_dependency_state((), lambda n: None)
|
|
self.assertFalse(result["dependency_unmet"])
|
|
self.assertIsNone(result["reason"])
|
|
|
|
|
|
class DependencyBlockedCandidateTest(unittest.TestCase):
|
|
"""A dependency-blocked candidate must be skipped, not selected."""
|
|
|
|
def test_classify_skip_rejects_unmet_dependency(self) -> None:
|
|
candidate = WorkCandidate(
|
|
kind="issue",
|
|
number=644,
|
|
labels=("status:ready",),
|
|
priority=20,
|
|
dependency_unmet=True,
|
|
dependency_reason="issue#644 depends on unresolved issue(s) #633",
|
|
)
|
|
reason = classify_skip(candidate, role="author", terminal_pr=None)
|
|
self.assertIsNotNone(reason)
|
|
self.assertIn("#633", reason)
|
|
|
|
|
|
class SelectionInvarianceTest(unittest.TestCase):
|
|
"""AC1/AC2/AC11: ranking sees everything; result bounds cannot move the winner."""
|
|
|
|
def setUp(self) -> None:
|
|
self._tmp = tempfile.TemporaryDirectory()
|
|
self.db = ControlPlaneDB(os.path.join(self._tmp.name, "cp.sqlite3"))
|
|
|
|
def tearDown(self) -> None:
|
|
self._tmp.cleanup()
|
|
|
|
@staticmethod
|
|
def _ready_issue(number: int, **kw) -> WorkCandidate:
|
|
return WorkCandidate(
|
|
kind="issue",
|
|
number=number,
|
|
labels=("status:ready",),
|
|
priority=20,
|
|
title=f"issue {number}",
|
|
**kw,
|
|
)
|
|
|
|
def _preview(self, candidates):
|
|
return allocate_next_work(
|
|
self.db,
|
|
session_id="s-758",
|
|
role="author",
|
|
remote="prgs",
|
|
org="Scaled-Tech-Consulting",
|
|
repo="Gitea-Tools",
|
|
candidates=candidates,
|
|
apply=False,
|
|
)
|
|
|
|
def test_more_than_fifty_candidates_lowest_number_wins(self) -> None:
|
|
"""Winner is the oldest eligible issue across a >50 inventory."""
|
|
candidates = [self._ready_issue(n) for n in range(600, 700)] # 100 items
|
|
result = self._preview(candidates)
|
|
self.assertEqual(result["outcome"], OUTCOME_PREVIEW)
|
|
self.assertEqual(result["selected"]["number"], 600)
|
|
|
|
def test_selection_is_invariant_to_candidate_ordering(self) -> None:
|
|
"""Ranking must not depend on the order the inventory arrived in."""
|
|
forward = [self._ready_issue(n) for n in range(600, 700)]
|
|
reverse = list(reversed(forward))
|
|
self.assertEqual(
|
|
self._preview(forward)["selected"]["number"],
|
|
self._preview(reverse)["selected"]["number"],
|
|
)
|
|
|
|
def test_truncating_inventory_changes_winner(self) -> None:
|
|
"""Regression guard: this is exactly what pre-ranking slicing did.
|
|
|
|
A 50-item slice of a 100-item inventory yields a different winner, so
|
|
any future reintroduction of pre-ranking truncation is detectable.
|
|
"""
|
|
full = [self._ready_issue(n) for n in range(600, 700)]
|
|
sliced = sorted(full, key=lambda c: -c.number)[:50]
|
|
self.assertNotEqual(
|
|
self._preview(full)["selected"]["number"],
|
|
self._preview(sliced)["selected"]["number"],
|
|
)
|
|
|
|
def test_blocked_first_candidate_falls_through_to_next(self) -> None:
|
|
"""AC8: a blocked winner must not end the iteration."""
|
|
blocked = self._ready_issue(
|
|
600,
|
|
dependency_unmet=True,
|
|
dependency_reason="issue#600 depends on unresolved issue(s) #599",
|
|
)
|
|
result = self._preview([blocked, self._ready_issue(601)])
|
|
self.assertEqual(result["selected"]["number"], 601)
|
|
skipped = {s["number"] for s in result["skipped"]}
|
|
self.assertIn(600, skipped)
|
|
|
|
def test_all_blocked_yields_no_safe_work(self) -> None:
|
|
candidates = [
|
|
self._ready_issue(
|
|
n, dependency_unmet=True, dependency_reason=f"issue#{n} blocked"
|
|
)
|
|
for n in range(600, 605)
|
|
]
|
|
result = self._preview(candidates)
|
|
self.assertIsNone(result["selected"])
|
|
self.assertEqual(len(result["skipped"]), 5)
|
|
|
|
def test_dry_run_and_apply_select_identically(self) -> None:
|
|
"""AC9: apply mode must not re-rank differently from preview."""
|
|
candidates = [self._ready_issue(n) for n in range(600, 700)]
|
|
preview = self._preview(candidates)
|
|
applied = allocate_next_work(
|
|
self.db,
|
|
session_id="s-758-apply",
|
|
role="author",
|
|
remote="prgs",
|
|
org="Scaled-Tech-Consulting",
|
|
repo="Gitea-Tools",
|
|
candidates=candidates,
|
|
apply=True,
|
|
)
|
|
self.assertEqual(
|
|
preview["selected"]["number"], applied["selected"]["number"]
|
|
)
|
|
|
|
def test_sort_is_stable_and_documented(self) -> None:
|
|
ordered = sort_candidates(
|
|
[self._ready_issue(603), self._ready_issue(601), self._ready_issue(602)]
|
|
)
|
|
self.assertEqual([c.number for c in ordered], [601, 602, 603])
|
|
self.assertIn("never affect selection", SELECTION_POLICY)
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|