fix(review): keep manual links out of the foreign repair and show dialog states
The repair tool could relink a manually linked row and drop its pair link. It now skips those rows and refuses an apply when one became manual after the plan. The picker's disabled action and chosen prompt are now visibly distinct.
This commit is contained in:
Vito Sansevero
@@ -642,6 +642,8 @@ test('#536: nothing selectable says why and links through the picker, then the n
|
||||
const negative = dialog.getByRole('combobox', { name: 'Negative prompt', exact: true });
|
||||
await expect(negative.locator('option')).toHaveText(['Choose a negative…', 'washed out', 'grainy', 'No negative']);
|
||||
await expect(submit).toHaveAttribute('aria-disabled', 'true');
|
||||
await expect(submit).toHaveCSS('opacity', '0.55');
|
||||
await expect(harbor).toHaveAttribute('aria-pressed', 'true');
|
||||
await negative.selectOption({ label: 'grainy' });
|
||||
const linked = page.waitForResponse(item => /\/link$/.test(item.url()));
|
||||
await submit.click();
|
||||
|
||||
@@ -285,6 +285,51 @@ class DryRunTests(RepairFixture):
|
||||
self.assertIn("would lose every link", markdown)
|
||||
|
||||
|
||||
class ManualLinkTests(RepairFixture):
|
||||
"""A prompt the reviewer named in Needs review (#536) is not ours to undo."""
|
||||
|
||||
def mark_manual(self, key):
|
||||
row = self.rows[key]
|
||||
((prompt,),) = self.query(
|
||||
"SELECT prompt_id FROM generated_images WHERE id=?", (row,)
|
||||
)
|
||||
self.query_write(
|
||||
"INSERT OR IGNORE INTO prompt_pairs(positive_prompt_id,created_at,"
|
||||
"updated_at) VALUES(?,'2026-10-30T00:00:00Z','2026-10-30T00:00:00Z')",
|
||||
(prompt,),
|
||||
)
|
||||
((pair,),) = self.query(
|
||||
"SELECT id FROM prompt_pairs WHERE positive_prompt_id=? "
|
||||
"AND negative_prompt_id IS NULL",
|
||||
(prompt,),
|
||||
)
|
||||
self.query_write(
|
||||
"INSERT INTO image_prompt_pairs(pair_id,image_id,provenance,created_at) "
|
||||
"VALUES(?,?,?,'2026-10-30T00:00:00Z')",
|
||||
(pair, row, '{"kind":"manual-link"}'),
|
||||
)
|
||||
|
||||
def test_a_manual_link_is_skipped_in_the_plan(self):
|
||||
self.mark_manual("A")
|
||||
code, out = self.dry_run()
|
||||
self.assertEqual(code, 0, out)
|
||||
items = {item["row_id"]: item for item in self.plan()["rows"]}
|
||||
item = items[self.rows["A"]]
|
||||
self.assertEqual(item["action"], "skip")
|
||||
self.assertEqual(item["reason"], "linked by hand in Needs review")
|
||||
self.assertEqual(items[self.rows["B"]]["action"], "relink")
|
||||
|
||||
def test_a_manual_link_made_after_the_plan_refuses_the_apply(self):
|
||||
code, out = self.dry_run()
|
||||
self.assertEqual(code, 0, out)
|
||||
self.mark_manual("A")
|
||||
before = self.links()
|
||||
code, out = self.apply()
|
||||
self.assertNotEqual(code, 0, out)
|
||||
self.assertIn("action changed", out)
|
||||
self.assertEqual(self.links(), before)
|
||||
|
||||
|
||||
class DryRunReviewTests(RepairFixture):
|
||||
def test_plan_records_its_library_and_fingerprint(self):
|
||||
self.dry_run()
|
||||
|
||||
@@ -112,6 +112,7 @@ COUNTED = (
|
||||
# The ladder steps whose answer is certain enough to keep a link.
|
||||
KEEP_SOURCES = ("recorded-run", "inspector")
|
||||
# What must be unchanged since the dry run for a row to be written.
|
||||
MANUAL_REASON = "linked by hand in Needs review"
|
||||
DRIFT_KEYS = (
|
||||
"action",
|
||||
"file",
|
||||
@@ -181,6 +182,13 @@ class Judge:
|
||||
)
|
||||
return self.decisions[path]
|
||||
|
||||
def _manually_linked(self, image_id):
|
||||
"""True when the reviewer named this row's prompt in Needs review (#536)."""
|
||||
return any(
|
||||
link["provenance"].get("kind") == "manual-link"
|
||||
for link in self.db.get_image_prompt_pairs(image_id)
|
||||
)
|
||||
|
||||
def row(self, row):
|
||||
"""The plan entry of one ``generated_images`` row."""
|
||||
conn = self.db.model.get_connection()
|
||||
@@ -200,6 +208,8 @@ class Judge:
|
||||
except OSError:
|
||||
return {**item, "action": "skip", "reason": "file gone"}
|
||||
item.update(file_size=stat.st_size, file_mtime_ns=stat.st_mtime_ns)
|
||||
if self._manually_linked(row["id"]):
|
||||
return {**item, "action": "skip", "reason": MANUAL_REASON}
|
||||
decision = self.decide(path, stat.st_size)
|
||||
action = action_of(decision, prompt_hash)
|
||||
if action == "skip":
|
||||
|
||||
+28
-28
@@ -43,14 +43,14 @@
|
||||
},
|
||||
"gallery-settings.js": {
|
||||
"lines": 98,
|
||||
"branches": 94,
|
||||
"branches": 95,
|
||||
"functions": 98,
|
||||
"statements": 98
|
||||
},
|
||||
"gallery.js": {
|
||||
"lines": 54,
|
||||
"branches": 72,
|
||||
"functions": 57,
|
||||
"functions": 58,
|
||||
"statements": 54
|
||||
},
|
||||
"html-escape.js": {
|
||||
@@ -210,10 +210,10 @@
|
||||
"statements": 98
|
||||
},
|
||||
"workspace/file-inspector.mjs": {
|
||||
"lines": 96,
|
||||
"lines": 97,
|
||||
"branches": 93,
|
||||
"functions": 93,
|
||||
"statements": 96
|
||||
"statements": 97
|
||||
},
|
||||
"workspace/heading.mjs": {
|
||||
"lines": 98,
|
||||
@@ -282,16 +282,10 @@
|
||||
"statements": 98
|
||||
},
|
||||
"workspace/images-delete.mjs": {
|
||||
"lines": 80,
|
||||
"branches": 70,
|
||||
"functions": 80,
|
||||
"statements": 80
|
||||
},
|
||||
"workspace/images-return.mjs": {
|
||||
"lines": 80,
|
||||
"branches": 70,
|
||||
"functions": 80,
|
||||
"statements": 80
|
||||
"lines": 95,
|
||||
"branches": 81,
|
||||
"functions": 92,
|
||||
"statements": 95
|
||||
},
|
||||
"workspace/images-export.mjs": {
|
||||
"lines": 98,
|
||||
@@ -311,6 +305,12 @@
|
||||
"functions": 98,
|
||||
"statements": 98
|
||||
},
|
||||
"workspace/images-return.mjs": {
|
||||
"lines": 93,
|
||||
"branches": 86,
|
||||
"functions": 98,
|
||||
"statements": 93
|
||||
},
|
||||
"workspace/images-selection.mjs": {
|
||||
"lines": 98,
|
||||
"branches": 93,
|
||||
@@ -349,7 +349,7 @@
|
||||
},
|
||||
"workspace/images.mjs": {
|
||||
"lines": 97,
|
||||
"branches": 89,
|
||||
"branches": 90,
|
||||
"functions": 98,
|
||||
"statements": 97
|
||||
},
|
||||
@@ -362,7 +362,7 @@
|
||||
"workspace/inspect.mjs": {
|
||||
"lines": 98,
|
||||
"branches": 92,
|
||||
"functions": 93,
|
||||
"functions": 94,
|
||||
"statements": 98
|
||||
},
|
||||
"workspace/inspection-state.mjs": {
|
||||
@@ -373,7 +373,7 @@
|
||||
},
|
||||
"workspace/inspector-collections.mjs": {
|
||||
"lines": 98,
|
||||
"branches": 92,
|
||||
"branches": 93,
|
||||
"functions": 98,
|
||||
"statements": 98
|
||||
},
|
||||
@@ -391,7 +391,7 @@
|
||||
},
|
||||
"workspace/library-editor.mjs": {
|
||||
"lines": 98,
|
||||
"branches": 93,
|
||||
"branches": 94,
|
||||
"functions": 92,
|
||||
"statements": 98
|
||||
},
|
||||
@@ -492,10 +492,10 @@
|
||||
"statements": 98
|
||||
},
|
||||
"workspace/more-menu.mjs": {
|
||||
"lines": 80,
|
||||
"branches": 70,
|
||||
"functions": 80,
|
||||
"statements": 80
|
||||
"lines": 98,
|
||||
"branches": 86,
|
||||
"functions": 98,
|
||||
"statements": 98
|
||||
},
|
||||
"workspace/plural.mjs": {
|
||||
"lines": 98,
|
||||
@@ -540,14 +540,14 @@
|
||||
"statements": 95
|
||||
},
|
||||
"workspace/review-link.mjs": {
|
||||
"lines": 80,
|
||||
"branches": 50,
|
||||
"functions": 70,
|
||||
"statements": 80
|
||||
"lines": 95,
|
||||
"branches": 70,
|
||||
"functions": 91,
|
||||
"statements": 95
|
||||
},
|
||||
"workspace/review-state.mjs": {
|
||||
"lines": 96,
|
||||
"branches": 84,
|
||||
"branches": 85,
|
||||
"functions": 98,
|
||||
"statements": 96
|
||||
},
|
||||
@@ -613,7 +613,7 @@
|
||||
},
|
||||
"workspace/system-sources.mjs": {
|
||||
"lines": 98,
|
||||
"branches": 88,
|
||||
"branches": 89,
|
||||
"functions": 95,
|
||||
"statements": 98
|
||||
},
|
||||
|
||||
@@ -71,7 +71,7 @@ export function openPromptLink({ api, scope, controller, filename, trigger }) {
|
||||
const warning = el('p', '', 'warning-note');
|
||||
warning.setAttribute('role', 'status');
|
||||
warning.hidden = true;
|
||||
const actions = el('div', undefined, 'dialog-actions');
|
||||
const actions = el('div', undefined, 'dialog-actions review-link-actions');
|
||||
const cancel = createButton(LINK_COPY.cancel);
|
||||
const submit = createButton(LINK_COPY.action, 'ca-primary');
|
||||
actions.append(cancel, submit);
|
||||
|
||||
@@ -438,7 +438,8 @@ pre.prompt-text { font: inherit; font-size: 12px; line-height: 1.75; margin-bloc
|
||||
.review-unreadable { display: grid; gap: 6px; margin-block: 12px; overflow-wrap: anywhere; }
|
||||
.review-link-results { display: grid; gap: 6px; max-block-size: 40vh; overflow-y: auto; }
|
||||
.review-link-results > button { justify-content: flex-start; inline-size: 100%; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; }
|
||||
.review-link-results > button[aria-pressed="true"] { border-color: var(--ca-accent); }
|
||||
.review-link-results > button[aria-pressed="true"] { border-color: var(--ca-accent); background: var(--soft-blue); }
|
||||
.review-panel button[aria-disabled="true"], .review-link-actions button[aria-disabled="true"] { opacity: .55; cursor: not-allowed; }
|
||||
/* Auto-tag selected images (#249 PR D, D85/D90): the Tag image dialog's model panel, then How to apply, a radio group
|
||||
in the concept's `.field` row whose radios keep their own size (the concept's `.field input` is a full-width text
|
||||
field) beside their label, as the concept's own radio rows (s1160 `.compare-item label`). */
|
||||
|
||||
Reference in New Issue
Block a user