fix(anchors): lève 2 nits revue — contrat crop corrompu + workflow_id sidecar — 2026-07-03
Suite revue adversariale MERGE-READY (« non-bloquant ≠ oublié », Dom) : 1. Contrat crop CORROMPU figé par test : `materialize_anchor_from_b64` retire la clé `_anchor_image_base64` de params AVANT le try (pop). Un crop inexploitable (b64 invalide, PNG au magic cassé) → pas d'exception, anchor_id NULL, aucun VisualAnchor, ET clé retirée (pas de donnée morte volumineuse dans les params). 3 cas paramétrés verrouillent ce comportement assumé. Docstring clarifiée. 2. `workflow_id` réintroduit dans le sidecar metadata.json (parité avec la route d'origine avant refactor). Nouveau paramètre optionnel du helper, passé par les deux appelants live (route HTTP + bridge R1). 2 tests : présent quand fourni, absent sinon (pas de clé None parasite). Backfill inchangé (workflow_id optionnel). TDD : +5 tests. 29 tests unit VERTS, 0 régression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -335,7 +335,9 @@ def import_learned_workflow(workflow_id: str):
|
||||
# NE PAS supprimer _anchor_bbox : conservé dans params pour que le
|
||||
# frontend lise x_pct/y_pct et affiche la zone ciblée.
|
||||
from services.anchor_image_service import materialize_anchor_from_b64
|
||||
materialize_anchor_from_b64(step, params, db_session=db.session)
|
||||
materialize_anchor_from_b64(
|
||||
step, params, db_session=db.session, workflow_id=wf_id
|
||||
)
|
||||
|
||||
step.parameters = params
|
||||
db.session.add(step)
|
||||
|
||||
@@ -197,6 +197,7 @@ def materialize_anchor_from_b64(
|
||||
*,
|
||||
db_session,
|
||||
source: str = "learned_import",
|
||||
workflow_id: Optional[str] = None,
|
||||
) -> Optional[str]:
|
||||
"""Matérialise une ancre visuelle à partir de `_anchor_image_base64`.
|
||||
|
||||
@@ -206,7 +207,10 @@ def materialize_anchor_from_b64(
|
||||
|
||||
Comportement, si `params` contient une clé `_anchor_image_base64` non vide :
|
||||
1. retire la clé de `params` (`pop`) → le crop volumineux n'est PAS
|
||||
re-stocké en double dans `parameters_json` ;
|
||||
re-stocké en double dans `parameters_json`. Le `pop` a lieu AVANT le
|
||||
`try` : un crop CORROMPU (b64 invalide, PNG au magic cassé) est
|
||||
inexploitable, donc retiré quand même (pas de re-tentative possible,
|
||||
éviter de traîner une donnée morte volumineuse dans les params) ;
|
||||
2. décode le b64 → PIL.Image (pour connaître les dimensions du crop) ;
|
||||
3. `save_anchor_image(...)` écrit `original.png` + `thumbnail.jpg` sur disque ;
|
||||
4. crée une ligne `VisualAnchor` (bbox = plein crop) + `db_session.add(...)` ;
|
||||
@@ -225,6 +229,8 @@ def materialize_anchor_from_b64(
|
||||
params: dict des paramètres du step (mutable — la clé b64 est retirée).
|
||||
db_session: session SQLAlchemy (transaction gérée par l'appelant).
|
||||
source: `capture_method` / marqueur de provenance (défaut learned_import).
|
||||
workflow_id: id du workflow VWB parent — écrit dans le sidecar
|
||||
`metadata.json` (champ informatif, parité avec la route d'origine).
|
||||
|
||||
Returns:
|
||||
L'`anchor_id` créé, ou None si aucun crop exploitable.
|
||||
@@ -245,11 +251,14 @@ def materialize_anchor_from_b64(
|
||||
# bbox plein crop : le b64 est DÉJÀ le crop de la zone cible (cf.
|
||||
# stream_processor crop 80×80), pas un screenshot plein écran.
|
||||
bbox = {"x": 0, "y": 0, "width": width, "height": height}
|
||||
anchor_meta = {"source": source}
|
||||
if workflow_id:
|
||||
anchor_meta["workflow_id"] = workflow_id
|
||||
result = save_anchor_image(
|
||||
anchor_id=anchor_id,
|
||||
image_base64=clean_b64,
|
||||
bounding_box=bbox,
|
||||
metadata={"source": source},
|
||||
metadata=anchor_meta,
|
||||
)
|
||||
if not result.get("success"):
|
||||
logger.warning(
|
||||
|
||||
@@ -453,7 +453,9 @@ def import_core_workflow_to_db(
|
||||
# `materialize_anchor_from_b64` retire la clé b64 de params (pas de
|
||||
# double stockage) et ne commit pas (transaction gérée ci-dessous).
|
||||
from services.anchor_image_service import materialize_anchor_from_b64
|
||||
materialize_anchor_from_b64(step, params, db_session=db_session)
|
||||
materialize_anchor_from_b64(
|
||||
step, params, db_session=db_session, workflow_id=wf_id
|
||||
)
|
||||
step.parameters = params
|
||||
db_session.add(step)
|
||||
|
||||
|
||||
@@ -351,3 +351,99 @@ def test_http_route_delegates_to_shared_helper(db_app, anchor_data_dir):
|
||||
assert step.anchor_id is not None
|
||||
assert VisualAnchor.query.get(step.anchor_id) is not None
|
||||
assert "_anchor_image_base64" not in step.parameters
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 7. Crop CORROMPU : contrat figé (pas d'exception, anchor_id NULL, clé retirée)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"corrupt_b64",
|
||||
[
|
||||
"!!! pas du base64 valide !!!", # base64 invalide
|
||||
"Zm9vYmFy", # b64 valide mais pas une image
|
||||
base64.b64encode(b"\x89PNG\x0d\x0a broken").decode("ascii"), # magic PNG cassé
|
||||
],
|
||||
ids=["b64_invalide", "b64_non_image", "png_magic_casse"],
|
||||
)
|
||||
def test_corrupt_crop_is_tolerated(db_app, anchor_data_dir, corrupt_b64):
|
||||
"""FIGE le contrat : un `_anchor_image_base64` corrompu est inexploitable.
|
||||
Le helper ne lève PAS, retourne None, laisse `anchor_id` NULL, ne crée
|
||||
aucun VisualAnchor — ET retire quand même la clé b64 de params (pop avant
|
||||
le try : un crop mort ne doit pas traîner, volumineux, dans les params)."""
|
||||
from services.anchor_image_service import materialize_anchor_from_b64
|
||||
|
||||
with db_app.app_context():
|
||||
wf = Workflow(id="wf_corrupt", name="wf", source="learned_import")
|
||||
db.session.add(wf)
|
||||
step = Step(id="step_corrupt", workflow_id="wf_corrupt",
|
||||
action_type="click_anchor", order=0, label="cible")
|
||||
db.session.add(step)
|
||||
|
||||
params = {"_anchor_image_base64": corrupt_b64, "target_text": "OK"}
|
||||
# Ne doit PAS lever malgré le crop corrompu.
|
||||
anchor_id = materialize_anchor_from_b64(
|
||||
step, params, db_session=db.session
|
||||
)
|
||||
|
||||
assert anchor_id is None, "crop corrompu → pas d'anchor_id"
|
||||
assert step.anchor_id is None, "anchor_id reste NULL"
|
||||
assert VisualAnchor.query.count() == 0, "aucun VisualAnchor créé"
|
||||
assert "_anchor_image_base64" not in params, \
|
||||
"clé b64 retirée même si le crop est inexploitable (pop avant try)"
|
||||
assert params.get("target_text") == "OK", "autres params préservés"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 8. workflow_id présent dans le sidecar metadata.json (parité route d'origine)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_workflow_id_written_to_sidecar_metadata(db_app, anchor_data_dir):
|
||||
"""Le `workflow_id` passé au helper est écrit dans le sidecar metadata.json
|
||||
(champ informatif — parité avec la route HTTP d'origine)."""
|
||||
from services.anchor_image_service import (
|
||||
materialize_anchor_from_b64,
|
||||
get_anchor_metadata,
|
||||
)
|
||||
|
||||
with db_app.app_context():
|
||||
wf = Workflow(id="wf_meta", name="wf", source="learned_import")
|
||||
db.session.add(wf)
|
||||
step = Step(id="step_meta", workflow_id="wf_meta",
|
||||
action_type="click_anchor", order=0)
|
||||
db.session.add(step)
|
||||
|
||||
params = {"_anchor_image_base64": _png_b64()}
|
||||
anchor_id = materialize_anchor_from_b64(
|
||||
step, params, db_session=db.session, workflow_id="wf_meta"
|
||||
)
|
||||
assert anchor_id is not None
|
||||
|
||||
meta = get_anchor_metadata(anchor_id)
|
||||
assert meta is not None, "metadata.json écrit"
|
||||
extra = meta.get("extra") or {}
|
||||
assert extra.get("workflow_id") == "wf_meta", \
|
||||
"workflow_id présent dans le sidecar (parité route d'origine)"
|
||||
assert extra.get("source") == "learned_import"
|
||||
|
||||
|
||||
def test_workflow_id_absent_when_not_provided(db_app, anchor_data_dir):
|
||||
"""Sans workflow_id fourni, la clé n'est pas écrite (pas de None parasite)."""
|
||||
from services.anchor_image_service import (
|
||||
materialize_anchor_from_b64,
|
||||
get_anchor_metadata,
|
||||
)
|
||||
|
||||
with db_app.app_context():
|
||||
wf = Workflow(id="wf_nom", name="wf", source="learned_import")
|
||||
db.session.add(wf)
|
||||
step = Step(id="step_nom", workflow_id="wf_nom",
|
||||
action_type="click_anchor", order=0)
|
||||
db.session.add(step)
|
||||
|
||||
anchor_id = materialize_anchor_from_b64(
|
||||
step, {"_anchor_image_base64": _png_b64()}, db_session=db.session
|
||||
)
|
||||
meta = get_anchor_metadata(anchor_id)
|
||||
extra = meta.get("extra") or {}
|
||||
assert "workflow_id" not in extra, "pas de clé workflow_id parasite"
|
||||
|
||||
Reference in New Issue
Block a user