fix(review): upgrade old default items of anchored files, tag anchored results (#390)
A pending default item recorded before the saver anchor no longer shortcuts the rescan when only the anchor resolves the file: the anchored item supersedes it. Apply accepts a resolved re-resolution only when it is tagged as the saver anchor's, the anchored pair must carry a positive text to choose, and limited anchored evidence still counts as a choice. TESTING.md describes the ruling.
This commit is contained in:
+14
-11
@@ -4595,17 +4595,20 @@ pending, undecided item of bytes now untraced leaves the queue
|
||||
(`settle_stale_review_items`), so an item never stays stuck at 409.
|
||||
Saver anchor (#390, `tests/test_review_saver_anchor.py`): the scan and Apply
|
||||
resolve with the file's name as the workspace inspector does, so the one saver
|
||||
whose `filename_prefix` names the file anchors its branch. A file whose branch
|
||||
that anchor resolves is never queued: from the inspection profile it offers no
|
||||
choice; a default-ambiguous file with a choice asks `saver_anchored` (the named
|
||||
run resolves, the unnamed one does not) and is reported `unresolved`, never
|
||||
linked. Its pending, undecided items leave on the next scan
|
||||
(`REVIEW_RULE_VERSION` is folded into inspection item versions; a default item
|
||||
is settled directly). No match, several matches or an unfinished walk anchor
|
||||
nothing, and the file stays queued as before. Apply refuses a choice the anchor
|
||||
contradicts. Measured on the maintainer's 40,890 outputs: the queue is
|
||||
unchanged (249 files); the 242 `gpu_unlock_bench` files the inspector resolves
|
||||
are linked by the parser floor, so they were never queued. The 16-candidate limited
|
||||
whose `filename_prefix` names the file anchors its branch. A file is never both
|
||||
unlinked and unqueued (maintainer ruling): when only that anchor resolves it
|
||||
(the named inspection run resolves, the unnamed one does not) and neither the
|
||||
default profile nor the parser floor links it, the scan queues it with the
|
||||
anchor's pair as its one candidate (evidence `anchor: "saver"`; the page
|
||||
preselects it), never linking it. Such an item counts as a choice for the
|
||||
untraced settle. Files already linked or resolved by the default path are
|
||||
unchanged. No match, several matches or an unfinished walk anchor nothing, and
|
||||
the file is queued as before. `REVIEW_RULE_VERSION` is folded into inspection
|
||||
item versions, so older inspection items are recorded again once. Apply accepts
|
||||
the anchored pair and refuses a choice the anchor contradicts. Measured on the
|
||||
maintainer's 40,890 outputs: the queue is unchanged (249 files); the 242
|
||||
`gpu_unlock_bench` files the inspector resolves are linked by the parser
|
||||
floor. The 16-candidate limited
|
||||
rule and the 8 MiB bound apply unchanged. The queue heals itself: after each batch, the scan removes the pending
|
||||
items of every listed file (read or skipped as known) that a saved row now
|
||||
links, and the monitor does the same for each ambiguous output it links
|
||||
|
||||
@@ -229,14 +229,14 @@ def evidence_offers_choice(evidence: Any) -> bool:
|
||||
"""
|
||||
if not isinstance(evidence, dict):
|
||||
return False
|
||||
# The saver anchor's one pair (#390) is a suggestion to confirm.
|
||||
needed = 1 if evidence.get("anchor") == "saver" else MIN_CHOOSABLE
|
||||
if evidence.get("limited"):
|
||||
total = evidence.get("candidate_total")
|
||||
return type(total) is int and total >= MIN_CHOOSABLE
|
||||
return type(total) is int and total >= needed
|
||||
candidates = evidence.get("candidates")
|
||||
if not isinstance(candidates, list):
|
||||
return False
|
||||
# The saver anchor's one pair (#390) is a suggestion to confirm.
|
||||
needed = 1 if evidence.get("anchor") == "saver" else MIN_CHOOSABLE
|
||||
return sum(1 for c in candidates if choosable(c)) >= needed
|
||||
|
||||
|
||||
|
||||
+2
-2
@@ -730,8 +730,8 @@ def _candidate_still_offered(resolution: dict, choice: str) -> bool:
|
||||
that rewords the top-level reason or reorders candidates leaves a
|
||||
pending item applicable.
|
||||
"""
|
||||
if isinstance(resolution, dict) and resolution.get("state") == "resolved":
|
||||
# The bytes now name one pair (the saver anchor, #390): only it.
|
||||
if isinstance(resolution, dict) and resolution.get("anchor") == "saver":
|
||||
# Only the saver anchor resolves the bytes (#390): its pair alone.
|
||||
resolution = {"state": "ambiguous", "candidates": [resolution]}
|
||||
if not isinstance(resolution, dict) or resolution.get("state") != "ambiguous":
|
||||
return False
|
||||
|
||||
@@ -19,6 +19,7 @@ try:
|
||||
from tests.test_review import unlinkable_graph, write_png
|
||||
from tests.test_review_inspection import (
|
||||
PROMPTS,
|
||||
_counted,
|
||||
InspectionTestCase,
|
||||
qwen_edit_graph,
|
||||
)
|
||||
@@ -26,6 +27,7 @@ except ImportError: # discovered with tests/ as the top-level directory
|
||||
from test_review import unlinkable_graph, write_png
|
||||
from test_review_inspection import (
|
||||
PROMPTS,
|
||||
_counted,
|
||||
InspectionTestCase,
|
||||
qwen_edit_graph,
|
||||
)
|
||||
@@ -265,6 +267,47 @@ class TestDefaultItemsAnchor(InspectionTestCase):
|
||||
self.assertEqual(data["code"], "review-conflict", data)
|
||||
self.assertIsNone(self._linked_text(path))
|
||||
|
||||
async def test_old_default_item_is_replaced_by_the_anchor_choice(self):
|
||||
self._roots()
|
||||
write_png(
|
||||
self.output_dir / "side_0_00001_.png",
|
||||
prompt=saved_unlinkable_graph("cat", "dog"),
|
||||
)
|
||||
with patch.object(review_capture, "_captured_inspection", return_value=None):
|
||||
await self._scan() # queued from the default profile, as before #390
|
||||
(old,) = self._items()
|
||||
self.assertEqual(old["resolver_version"], review_capture.RESOLVER_VERSION)
|
||||
|
||||
await self._scan("all")
|
||||
|
||||
(item,) = self._items()
|
||||
self.assertNotEqual(item["id"], old["id"])
|
||||
self.assertEqual(_only_candidate(self, item), "cat")
|
||||
|
||||
with _counted(review_capture, "resolve_inspection") as calls:
|
||||
await self._scan("all") # now queued under its version: no re-read
|
||||
self.assertEqual(calls, [])
|
||||
self.assertEqual([i["id"] for i in self._items()], [item["id"]])
|
||||
|
||||
async def test_decided_anchored_item_is_kept(self):
|
||||
self._require_routes()
|
||||
self._roots()
|
||||
write_png(
|
||||
self.output_dir / "side_0_00001_.png",
|
||||
prompt=saved_unlinkable_graph("cat", "dog"),
|
||||
)
|
||||
await self._scan()
|
||||
item, detail, observed = await self._open()
|
||||
status, data = await self._decide(
|
||||
item["id"], self._body(detail, observed, "unresolved")
|
||||
)
|
||||
self.assertEqual(status, 200, data)
|
||||
before = (self._items(), self._decisions())
|
||||
|
||||
await self._scan("all")
|
||||
|
||||
self.assertEqual((self._items(), self._decisions()), before)
|
||||
|
||||
async def test_apply_accepts_an_old_default_choice_the_anchor_names(self):
|
||||
self._require_routes()
|
||||
self._roots()
|
||||
|
||||
+50
-17
@@ -213,17 +213,28 @@ def inspection_choice(metadata: dict, file_name: str | None = None) -> dict | No
|
||||
return None
|
||||
resolution = resolve_inspection(metadata, file_name)
|
||||
if resolution.get("state") == "resolved":
|
||||
return resolution if _anchor_alone(metadata, file_name) else None
|
||||
return _anchor_of(metadata, file_name, resolution)
|
||||
if resolution.get("state") != "ambiguous" or not reviewable(resolution):
|
||||
return None
|
||||
return resolution
|
||||
|
||||
|
||||
def _anchor_alone(metadata: Any, file_name: str | None) -> bool:
|
||||
"""True when ``metadata`` does not resolve without ``file_name``."""
|
||||
if not file_name:
|
||||
return False
|
||||
return resolve_inspection(metadata).get("state") != "resolved"
|
||||
def _anchor_of(metadata: Any, file_name: str | None, named: dict) -> dict | None:
|
||||
"""``named`` tagged ``anchor: "saver"`` when only the saver anchor
|
||||
resolves the file, to a pair with a positive text to choose; else None.
|
||||
|
||||
``named`` is ``metadata`` resolved with ``file_name``; the profile runs
|
||||
once more without the name only when it is resolved and choosable.
|
||||
"""
|
||||
if not file_name or named.get("state") != "resolved":
|
||||
return None
|
||||
try:
|
||||
offered = choosable(_projection(named))
|
||||
except (KeyError, TypeError):
|
||||
return None
|
||||
if not offered or resolve_inspection(metadata).get("state") == "resolved":
|
||||
return None
|
||||
return {**named, "anchor": SAVER_ANCHOR}
|
||||
|
||||
|
||||
def saver_anchored(metadata: Any, file_name: str | None) -> bool:
|
||||
@@ -245,10 +256,7 @@ def _anchored(metadata: Any, file_name: str | None) -> dict | None:
|
||||
return None
|
||||
if inspection_review_version(metadata) is None:
|
||||
return None
|
||||
resolution = resolve_inspection(metadata, file_name)
|
||||
if resolution.get("state") != "resolved":
|
||||
return None
|
||||
return resolution if _anchor_alone(metadata, file_name) else None
|
||||
return _anchor_of(metadata, file_name, resolve_inspection(metadata, file_name))
|
||||
|
||||
|
||||
def item_profile(evidence: Any) -> str:
|
||||
@@ -311,7 +319,9 @@ def record_ambiguous_file(
|
||||
"""
|
||||
versions = item_versions(scanned[0] if scanned is not None else None)
|
||||
spelled_path, queued = _queued_spelling(db, root, relative, versions)
|
||||
if queued is not None:
|
||||
if queued is not None and not (
|
||||
inspection and _default_item_to_anchor(db, queued, scanned, relative)
|
||||
):
|
||||
return _already_queued(db, *queued, versions)
|
||||
epoch = db.get_execution_epoch()
|
||||
try:
|
||||
@@ -338,7 +348,7 @@ def record_ambiguous_file(
|
||||
return "skipped"
|
||||
profile, version = DEFAULT, RESOLVER_VERSION
|
||||
named = _captured_inspection(captured[0], relative, scanned) if inspection else None
|
||||
anchored = named is not None and named[0].get("state") == "resolved"
|
||||
anchored = named is not None and named[0].get("anchor") == SAVER_ANCHOR
|
||||
if anchored:
|
||||
# Only the saver anchor resolves it (#390): offer that one pair.
|
||||
profile, version = INSPECTION, named[1]
|
||||
@@ -378,13 +388,31 @@ def record_ambiguous_file(
|
||||
return "limited" if limited else "recorded"
|
||||
|
||||
|
||||
def _default_item_to_anchor(db, queued: tuple, scanned, relative: str) -> bool:
|
||||
"""True when the bytes' only pending item is a default item recorded
|
||||
before #390 and the saver anchor alone resolves the scanned records.
|
||||
|
||||
The file is then captured again so the anchored item supersedes that
|
||||
pending, undecided item. Decided bytes and bytes already queued under
|
||||
the current inspection version are never re-read.
|
||||
"""
|
||||
if scanned is None or scanned[0] is None:
|
||||
return False
|
||||
version = inspection_review_version(scanned[0])
|
||||
if version is None or db.review_pending_at(*queued, version):
|
||||
return False
|
||||
if not db.review_pending_at(*queued, RESOLVER_VERSION):
|
||||
return False
|
||||
return _anchored(scanned[0], _file_name(relative)) is not None
|
||||
|
||||
|
||||
def _captured_inspection(metadata: dict, relative: str, scanned) -> tuple | None:
|
||||
"""``(resolution, item_version)`` of the captured records under the
|
||||
inspection profile, anchored on the file's name; None when the table is
|
||||
unusable.
|
||||
|
||||
A ``resolved`` result means the saver anchor alone resolves the file
|
||||
(#390); a result that also resolves without the name is returned as
|
||||
A result tagged ``anchor: "saver"`` means the saver anchor alone
|
||||
resolves the file (#390); any other resolved result is returned as
|
||||
``unresolved`` (no review case, as before). The scan's result is reused
|
||||
when the capture holds the same records (``inspection_choice`` applies
|
||||
the same rule).
|
||||
@@ -400,8 +428,12 @@ def _captured_inspection(metadata: dict, relative: str, scanned) -> tuple | None
|
||||
return scanned[1], version # the same records: no second run
|
||||
name = _file_name(relative)
|
||||
resolution = resolve_inspection(metadata, name)
|
||||
if resolution.get("state") == "resolved" and not _anchor_alone(metadata, name):
|
||||
resolution = {**resolution, "state": "unresolved"}
|
||||
if resolution.get("state") == "resolved":
|
||||
# Resolved without the anchor too: no review case, as before.
|
||||
resolution = _anchor_of(metadata, name, resolution) or {
|
||||
**resolution,
|
||||
"state": "unresolved",
|
||||
}
|
||||
return resolution, version
|
||||
|
||||
|
||||
@@ -491,7 +523,8 @@ def captured_review(
|
||||
info["format"] = read.format
|
||||
canonical["file_info"] = info
|
||||
if profile == INSPECTION:
|
||||
return canonical, resolve_inspection(canonical, file_name)
|
||||
named = resolve_inspection(canonical, file_name)
|
||||
return canonical, _anchor_of(canonical, file_name, named) or named
|
||||
resolution = resolve_prompt_pair(canonical)
|
||||
if file_name is not None and resolution.get("state") == "ambiguous":
|
||||
anchored = _anchored(canonical, file_name)
|
||||
|
||||
Reference in New Issue
Block a user