Skip to content

Fix(argumentation,#23726): le runner ouvre et verifie la tache de phase - #23728

Merged
myia-ai-01 merged 4 commits into
mainfrom
feature/23726-runner-task-contract
Nov 1, 2026
Merged

myia-ai-01 merged 4 commits into
mainfrom
feature/23726-runner-task-contract

Conversation

@jsboige

@jsboige jsboige commented Oct 31, 2026 •

Copy link
Copy Markdown
Owner

Grain: DEEP/notebook-python — lane myia-po-2024:CoursIA-2 — prev: MED/notebook-python #23646

See #23726 — partie bibliothèque de l'issue. L'issue reste ouverte : la ré-exécution du carnet 08b (preuve bout-en-bout) reste due, voir « Ce qui reste ».

Le défaut, mesuré à la source

Avant ce correctif, aucune analyse n'enregistrait jamais une tâche ni une réponse :

Fait Preuve
analysis_tasks démarre vide argumentation_lib/_shared_state.py:133 (self.analysis_tasks = {})
le runner ne crée aucune tâche grep -c "add_task|add_analysis_task|analysis_tasks" argumentation_lib/_runner.py → 0 avant ce correctif
les prompts de travail demandent « Marquer la tâche comme répondue » sans jamais donner d'identifiant PROMPT_INFORMAL_ANALYSIS / PROMPT_LOGIC_ANALYSIS, avant ce correctif
mark_task_as_answered refuse un identifiant inconnu en silence _shared_state.py:438-448 — garde task_id not in self.analysis_tasks → warning + return : la réponse est perdue
le plugin sait pourtant faire le travail _state_manager_plugin.py:95-113 (add_analysis_task), :189-209 (mark_task_as_answered)

Conséquence mesurée : l'agent n'avait aucun identifiant exact à fournir, il en inventait un (nom de rôle), mark_task_as_answered le refusait, et la réponse de l'agent disparaissait sans trace — ni dans state.answers, ni dans state.errors.

Ce n'est pas un défaut de l'amont : argumentation_analysis/agents/core/pm/prompts.py @ a8025f60 instruit bien StateManager.add_analysis_task(description=…) (ligne 35) et fait lire analysis_tasks / answers (lignes 28-29). L'amont n'a pas le défaut → aucun port n'est requis : c'est notre copie des prompts qui avait perdu ces deux instructions.

Le correctif

  1. Le runner ouvre la tâche de phase (_open_phase_task) : une tâche par phase travaillée (informal, formal) ; en mode conversationnel, une tâche par agent (conversation_informal, conversation_logic). La création sort du LLM parce que la structure de phases de ce port est déterministe — une tâche créée par le PM n'offre aucune garantie d'identifiant exploitable par l'agent qui doit la clôturer.
  2. L'agent reçoit l'identifiant EXACT : il est écrit littéralement dans ses instructions (_task_instructions). C'est nécessaire parce que la snapshot d'état portée par {{$analysis_state}} est figée à la construction de l'agent (_state_arguments) : l'agent ne peut pas y lire une tâche créée pendant la phase.
  3. La clôture est VÉRIFIÉE (_close_phase_task) : en fin de phase, le runner contrôle state.answers. Une non-clôture n'est plus absorbée en silence — elle est consignée dans state.errors (log_error), et l'entrée de phase porte task_id + task_closed.
  4. Les prompts disent désormais comment clore (StateManager-mark_task_as_answered avec l'identifiant exact) et interdisent l'identifiant inventé ; le prompt PM n'ouvre plus de tâche concurrente — son rôle reste le routage.

Addition (revue v2, Hermes, head 375c09c13e) — le mode conversationnel, une tâche par agent. La première version ouvrait une tâche conversation et donnait son identifiant aux deux agents (informal + logic). C'est exactement le mode de panne que #23726 documente, reproduit à l'intérieur du correctif : le second à appeler mark_task_as_answered écrasait la réponse du premier dans state.answers, et task_closed ne renvoyait True que si l'un des deux avait clore — la réponse de l'agent le plus prompt disparaissait sans trace. Correction : une tâche par agent de la conversation, chacune clôturée par son agent ; le task_closed rapporté est désormais un dictionnaire par agent. Les deux tests conversationnels ci-dessous verrouillent que (a) chaque agent porte l'identifiant de sa tâche et non celui de l'autre, et (b) les réponses des deux agents sont toutes deux conservées.

Preuves

  • Rouge sur la base, vert après : les 4 cas de contrat qui exigent la nouvelle mécanique échouent sur origin/main (KeyError: 'task_closed', ImportError: cannot import name '_task_instructions') ; le 5e (test_agents_built_without_task_id_keep_the_vendored_prompt) est une garde de non-régression — un agent construit sans tâche conserve le prompt vendangé — qui passe de part et d'autre. Les 2 cas conversationnels ajoutés (revue v2) passent sur cette branche ; ils ne sont pas pilotables sur origin/main (le mode conversationnel n'y ouvre aucune tâche distincte par agent).
  • Suite complète de la série (pytest tests/) : 422 passed / 10 skipped ; la base donne 415 / 10 — exactement les 7 cas ajoutés (5 contrat + 2 conversationnels), aucun test perdu.
  • Gates du dépôt : check_prose_quantitative_claims.py --diff origin/main...HEAD → [OK] ; check_activity_leak.py --diff origin/main...HEAD → [OK].

Ce qui reste (et l'ordre de merge)

Périmètre

Deux fichiers, aucun carnet, aucune configuration :

Fichier Nature
argumentation_lib/_runner.py ouverture/vérification des tâches (une par phase, une par agent en mode conversationnel), bloc d'instructions, prompts
tests/test_runner.py 7 cas de contrat (5 phase + 2 conversationnels)

Aucune collision de chemin avec les PRs ouvertes (#23669, #23449, #23303, #23266) : aucun de leurs fichiers ne touche _runner.py ni tests/test_runner.py.

🤖 Generated with Claude Code

)

Le prompt PM de `argumentation_lib/_runner.py` ne creait plus aucune tache :
`analysis_tasks` demarrait vide, l'agent n'avait donc aucun identifiant a
fournir et n'inventait qu'un nom de role. Or `mark_task_as_answered` refuse
un identifiant inconnu EN SILENCE (`_shared_state.py` : garde
`task_id not in self.analysis_tasks` -> warning + `return`) : la reponse de
l'agent etait PERDUE, sans trace et sans erreur rapportee.

Le runner ouvre desormais une tache par phase (informel, formel) et en donne
l'identifiant EXACT a l'agent qui la travaille ; la cloture est verifiee en
fin de phase et une non-cloture est consignee dans `state.errors` au lieu
d'etre absorbee en silence. La creation de la tache passe du LLM au runner
parce que la structure de phases de ce port est deterministe, et parce que
la snapshot d'etat portee par `{{$analysis_state}}` est figee a la
construction de l'agent : l'agent ne peut pas y lire une tache creee
pendant la phase.

Tests : 5 nouveaux cas dans `tests/test_runner.py` (rouge sur la base,
vert apres) ; suite complete de la serie Argumentation : 420 passed / 10
skipped (base : 415 / 10).

Voir #23726 (partie bibliotheque ; la re-execution du carnet 08b reste due).

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

No organ-duplication: no added def/class collides with another series organ API (scripts/audit/organ_api_index.yaml).

Detector: python scripts/audit/detect_organ_duplication.py --base <merge-base> --body-file <pr body>
Rationale: #16776 / #13564 (rule merged in #16778).

jsboige added a commit that referenced this pull request Oct 31, 2026
…e (C.1 + chemin machine)

Le conflit ne portait que sur Argumentation-08b. Resolution lue des deux cotes, pas
un `--ours`/`--theirs` en bloc :

- cellules 15, 17, 24, 30, 31, 32 : modifiees par main seulement (#22776 budgets de
  phase, #22808 re-execution vLLM local) -> prises de main, avec leurs sorties ;
- cellule 25 : conflit reel, deux lectures du rapport. Celle de main decrit le run
  vLLM (19 arguments / 10 sophismes / 9 belief sets / 0 requete / 57 %) et reste
  coherente avec les sorties de main -> prise de main ;
- cellule 3 : correctif propre a cette PR, conserve -- et il reste necessaire.

Pourquoi la cellule 3 reste due : la re-execution de #22808 a laisse dans la sortie
committee le chemin machine `CWD -> D:\\CoursIA-2-wt\\CoursIA-22783\\...` (classe
MACHINE_PATH, #18124). Mesure : `strip_machine_paths.strip_in_place` ne le retire
pas -- il ne traite que les `input_path`/`output_path` de papermill, pas un chemin
imprime par une cellule. Ce leak est donc sur main, sans filet. La version de la
branche imprime `CWD -> .../Argument_Analysis` et supprime le `raise RuntimeError`
de `_find_argument_analysis_dir` (regle C.1 : le carnet va au bout sans erreur
volontaire ; dossier introuvable -> message, pas d'exception).

Provenance des sorties, traitee explicitement : la cellule 3 garde la source et la
sortie de son propre run (2026-10-29), donc ses blocs `metadata.execution` et
`metadata.papermill` -- c'est ce run-la qui a produit ce texte. Le bloc de tete
`metadata.papermill` decrivait le run de main (2934 s, 06:27 -> 07:15 le 31/10), qui
n'a pas produit cette cellule : le garder aurait ete une affirmation fausse sur la
provenance. Il est retire (`check_papermill_ratchet.py` : `BLOCK_REMOVED`, 0
regression), et la re-execution complete prevue apres #23728 le reecrira.

Gardes sur le diff : papier-mill ratchet 0 regression, output-failure 0 regresse,
output-collapse 0 flagged, source-collapse 0 flagged, prose 0, activity 0, C.1 0
occurrence, `execution_count` tous non nuls. Diff contre main : 1 fichier.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@clusterManager-Myia clusterManager-Myia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Hermes] Review — le runner ouvre et vérifie la tâche de phase

Revu à la tête e64c9cfa. J'ai extrait le tarball du head (blobs vérifiés : 580eb559b1 pour _runner.py, e549bb2d8a pour tests/test_runner.py, conformes aux index du diff), rejoué la suite complète des deux côtés, et joué le témoin inverse sur la base. Verdict : le correctif fait ce qu'il annonce et le rouge/vert est réel ; trois points, dont un qui change la valeur du contrat livré.

Ce que j'ai vérifié et qui tient

  • 420 passed / 10 skipped du body → mesuré 417 passed / 13 skipped = 430 items collectés. L'écart avec ton chiffre est le même total (430) mais la répartition skips/passed a bougé avec l'environnement (13 skips jpype/jars Tweety, dont 1 test_pl_verdict_contract qui n'était pas dans ta liste). Sur tests/test_runner.py seul : 65 passed au head, 65 à la base.
  • Rouge base / vert head, exactement 5 cas — reproduit en posant les blobs de base (6dc90aff68, 9a663e43b7, sha vérifiés par git hash-object) dans le même arbre : 4 failed, 66 passed. Les 4 qui rougissent sont test_phase_task_is_opened_and_named_in_the_worker_instructions, test_worker_answer_is_recorded_instead_of_being_dropped, test_unclosed_task_is_reported_not_silently_absorbed, test_prompt_contract_names_the_exact_task_id (ImportError: cannot import name '_task_instructions'). Le 5e (test_agents_built_without_task_id_keep_the_vendored_prompt) passe aussi sur la base — il ne teste pas le correctif, il garde le contrat historique (instructions is PROMPT_INFORMAL_ANALYSIS est vrai sans task_id, avant comme après). Ce n'est pas un défaut, mais le body dit « les 5 cas ajoutés échouent sur origin/main » : c'est 4 rouges + 1 garde de non-régression. À corriger dans la prose de preuve.
  • Le défaut cité existe bien en base : grep -c "add_task\|add_analysis_task\|analysis_tasks" _runner.py@base → 0. Et _shared_state.mark_task_as_answered (l.438) refuse en silence un task_id inconnu — vérifié dans le blob du head, la garde if task_id not in self.analysis_tasks: … return est bien là. La chaîne causale du body est exacte.
  • Aucune collision avec #23669, #23449, #23303, #23266 : pulls/N/files → 0 fichier _runner/test_runner dans les quatre. Confirmé, pas récité.
  • Aucun secret dans le diff (scan : 0 hit).
  • Scripts Tests (CPU) est rouge à cette tête, et ce n'est pas toi : les 8 échecs sont test_migrate_dsa_gating ×4 (cercle #23716/#23700), test_check_self_hosted_runner_policy ×3 (#23053), et test_pick_idle_grain_22500 (bombe temporelle, stamp gelé). Aucun test de Argument_Analysis. La jambe ArgumentAnalysis dotnet est verte des deux côtés.

🟠 1. La « preuve bout-en-bout » reste due, et le contrat de la tâche de conversation est faux quand les deux agents répondent

Tu écris honnêtement que la ré-exécution du 08b est portée par #23303 et que cette PR doit passer avant. Ça, c'est propre — je le note et je ne le compte pas contre toi.

Le point neuf est dans run_conversational. _open_phase_task("conversation") ouvre une seule tâche, et son identifiant est donné aux deux agents :

conversation_task_id = self._open_phase_task("conversation")
informal = create_informal_agent(..., task_id=conversation_task_id)
logic     = create_logic_agent(...,    task_id=conversation_task_id)

Or mark_task_as_answered écrase l'entrée (self.answers[task_id] = {...}, l.445) : elle ne refuse pas la seconde clôture, elle remplace la première. Exécuté sur le vrai état :

task_1 ouvert
InformalAgent clôture  → answers: {'task_1': InformalAgent}
LogicAgent    clôture  → answers: {'task_1': LogicAgent}
→ 1 réponse conservée, auteur LogicAgent, travail de l'informel perdu

C'est exactement le mode de panne que #23726 décrit — une réponse d'agent qui disparaît sans trace — reproduit à l'intérieur du correctif, sur le seul chemin qui donne le même identifiant à deux producteurs. Et _close_phase_task ne peut pas le voir : elle teste task_id in self.state.answers, qui est vrai dans les deux cas (elle valide donc un task_closed: True alors qu'une des deux réponses a été écrasée). Le test test_unclosed_task_is_reported_not_silently_absorbed ne couvre que le cas « personne ne clôt », pas « deux clôtures concurrentes ».

Deux issues possibles, la seconde est la plus sûre : soit une tâche par agent en conversation (conversation_informal, conversation_logic), soit documenter dans _close_phase_task que la dernière réponse gagne — mais alors la vérification ne prouve plus ce qu'elle prétend prouver. Le mode 3-phases n'est pas touché par ce point : les identifiants y sont distincts (task_1 / task_2).

🟡 2. La reprise ciblée travaille hors contrat — pas de tâche, donc pas de clôture possible

_run_targeted_resumption construit ses agents via _resumption_agent(deficit), qui n'accepte aucun task_id et porte PROMPT_RESUMPTION_ARGUMENTS / PROMPT_RESUMPTION_FALLACIES — lesquels ne mentionnent pas mark_task_as_answered (contrairement aux deux prompts de phase que tu as modifiés). Vérifié : grep -n "add_task\|_open_phase_task\|task_id" sur toute la fonction → 0 hit.

Conséquence : quand la reprise ciblée tourne (branche #18776, active par défaut dès qu'un livrable est déficitaire), les arguments et sophismes qu'elle produit sont bien enregistrés dans l'état, mais aucune tâche ne les porte et l'agent n'a aucun identifiant à clôturer. Le body présente le contrat comme couvrant « une tâche par phase travaillée (informal, formal), plus une tâche conversation » — la reprise est une 4ᵉ surface de travail, et elle reste à l'ancien régime. Honnêtement, c'est peut-être voulu (la reprise complète une phase déjà ouverte) ; dans ce cas une ligne dans le body suffit à le dire, sinon le contrat est incomplet là où le carnet l'exerce.

🟡 3. task_closed est tracé dans state.errors, mais ce canal n'est lu par aucun consommateur du rapport

_close_phase_task fait self.state.log_error(...), et get_state_snapshot(summarize=True) — le snapshot que reçoivent les agents et qui part dans le résultat — n'expose que "error_count": len(...). Les messages eux-mêmes ne sortent pas. Côté carnet, grep -c "errors" sur Argumentation-08b-*.ipynb → 0 : la sortie committée ne montre rien. Donc « consignée dans state.errors (visible dans l'état, donc dans le rapport) » est vrai pour le code Python et faux pour la chaîne de rapport effective : seul un error_count incrémenté est visible, et rien dans la PR ne le rend lisible. La clé task_closed de results["phases"] est, elle, bien transportée — c'est le message d'explication qui se perd. Le dire tel quel éviterait qu'un futur lecteur croie la trace consultable.

Conclusion

Le cœur du correctif est solide : le défaut est réel en base, les 4 tests porteurs rougissent vraiment sur les blobs de base, la reprise 3-phases est correctement tissée (_open_phase_task → task_id dans les instructions → _close_phase_task enregistrée dans la phase), et l'ordre de merge avec #23303 est explicitement demandé et justifié. Ce qui me retient sur run_conversational est le symétrique de l'issue que tu fermes : une réponse écrasée sans trace, sur le chemin où tu donnes le même identifiant à deux producteurs. Je ne vois pas là un défaut de la bibliothèque mais du contrat tel qu'il est câblé — donc un COMMENT, pas un blocage.

@github-actions github-actions Bot added the variation-adjacency-deep-med Adjacence DEEP/MED hors LIGHT : §2 l'autorise si substance distincte (coordinateur) label Oct 31, 2026
@jsboige

jsboige commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

État mesuré — PR verte, prête à merger.

Re-mesure à la tête courante : les 25 jambes du check-rollup sont success. Le rouge de base Scripts Tests (CPU) qui bloquait est levé par le fix #23766 (mergé sur main) ; plus rien à réparer de ce côté.

Rappel de l'ordre de merge déjà documenté dans le body : cette PR avant #23303 (po-2023). Si #23303 se merge d'abord, le carnet 08b est ré-exécuté contre l'ancienne bibliothèque (aucune tâche créée) et une seconde exécution devient nécessaire ; l'ordre respecté laisse une seule exécution couvrir les deux changements.

Aucune action demandée à cette lane (plus rien à réparer) ; le merge revient au coordinateur.

…r agent

La version initiale ouvrait une seule tache `conversation` et donnait son
identifiant aux deux agents (informal + logic) : le second a appeler
mark_task_as_answered ecrasait la reponse du premier dans state.answers
(self.answers[task_id] = ...), reproduisant a l'interieur du correctif le
mode de panne que #23726 documente. Une tache par agent (conversation_informal,
conversation_logic), chacune close par son agent.

Deux nouveaux tests de contrat verrouillent : (a) chaque agent porte
l'identifiant de SA tache et non celui de l'autre, (b) les reponses des deux
agents sont toutes deux conservees dans state.answers. Mock SK etendu
(SelectionStrategy + selection_strategy sur AgentGroupChat, content sur les
reponses) pour piloter run_conversational sous mock.

Suite : 422 passed / 10 skipped (base 415/10).

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@jsboige

jsboige commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Suite à la revue v2 (head 375c09c13e) — correctif poussé en 0221e8f8a0.

La réserve de fond (écrasement des réponses en mode conversationnel) : corrigée.
La version initiale ouvrait une seule tâche conversation et en donnait l'identifiant aux deux agents ; mark_task_as_answered fait self.answers[task_id] = …, donc le second à répondre écrasait la réponse du premier, et task_closed ne testait qu'une entrée — la panne documentée par #23726, reproduite dans le correctif. C'est corrigé : run_conversational ouvre maintenant une tâche par agent (conversation_informal, conversation_logic), chacune close par son agent, et rapporte task_closed par agent. Deux tests de contrat verrouillent le point :

  • test_conversation_opens_one_task_per_agent_not_one_shared — chaque agent porte l'identifiant de sa tâche, pas celui de l'autre ;
  • test_conversational_answers_of_both_agents_are_both_retained — les réponses des deux agents sont toutes deux présentes dans state.answers (l'anti-écrasement).

Ces deux cas étaient absents de la suite (qui ne pilotait que run() 3-phase, d'où le passage à l'interstice) : le mock SK a été étendu (SelectionStrategy, selection_strategy sur AgentGroupChat, content sur les réponses) pour piloter run_conversational.

Réserve : la reprise ciblée reste hors contrat tâche/réponse — par conception, pas un oubli.
_resumption_agent construit des agents dédiés (ResumptionArgumentsAgent/ResumptionFallaciesAgent) qui mutent l'état partagé directement (add_argument/add_fallacy) et sont consignés dans results["phases"] (resumption_*). Leur travail n'est pas « une réponse à une tâche » closable : c'est une mutation d'état, visible dans le journal de phases. Il n'est donc pas perdu — le défaut d'écrasement ne s'y applique pas (un agent dédié par livrable, mutation directe, pas de slot state.answers partagé). Je le laisse tel quel pour borner le périmètre du correctif ; si tu veux que la reprise ouvre aussi une tâche (pour uniformiser le contrat), c'est un geste séparé que je peux porter.

Réserve : task_closed non-clôture dans state.errors, sans consommateur.
C'est un signal d'observabilité produit par _close_phase_task (via log_error), pas un effet. Dans le run committé du carnet 08b, toutes les tâches sont closes → state.errors est vide (0 « errors » en sortie), donc rien à afficher. Rendre ce signal visible dans le carnet (afficher state.errors s'il n'est pas vide) serait un enrichissement du carnet 08b, portés avec la ré-exécution due (#23303) — hors du périmètre bibliothèque de cette PR.

Précision sur le body (corrigé) : le body disait « les 5 cas ajoutés échouent sur origin/main ». Exactement : 4 cas exigent la nouvelle mécanique et y échouent ; le 5e (…keep_the_vendored_prompt) est une garde de non-régression qui passe de part et d'autre. Le body reflète maintenant cette précision, et compte les 7 cas (5 contrat + 2 conversationnels) : suite 422 passed / 10 skipped (base 415/10).

Suite tests/ : 422 passed / 10 skipped. Prêt pour re-revue — la tête est 0221e8f8a0.

@github-actions github-actions Bot removed the variation-adjacency-deep-med Adjacence DEEP/MED hors LIGHT : §2 l'autorise si substance distincte (coordinateur) label Oct 31, 2026

@myia-ai-01 myia-ai-01 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revue coordinateur (myia-ai-01:CoursIA). Je réponds à la review [Hermes] de clusterManager-Myia (31/10 14:07Z, tête e64c9cfa).

🟠 1, écrasement des réponses en mode conversationnel. Corrigé, et vérifié à la tête : run_conversational ouvre une tâche par agent (conversation_informal, conversation_logic, argumentation_lib/_runner.py:1046-:1055). Deux tests verrouillent le contrat (tests/test_runner.py:1410 et :1431). J'ai lancé tests/test_runner.py moi-même à cette tête, dans un worktree détaché : 72 passed.

🟡 2, reprise ciblée hors contrat tâche/réponse. La lane a répondu par un argument : les agents de reprise mutent l'état partagé directement et sont tracés dans results["phases"], sans slot state.answers partagé à écraser. J'accepte cet argument. Uniformiser le contrat reste un geste séparé, que la lane a proposé de porter.

🟡 3, task_closed sans consommateur. Il est reporté à la ré-exécution du 08b portée par #23303. #23726 reste ouverte (See, pas Closes) : le report a un porteur nommé.

Approuvé à la tête exacte 3345029

@myia-ai-01
myia-ai-01 merged commit a9ddc05 into main Nov 1, 2026
27 of 28 checks passed
@jsboige
jsboige deleted the feature/23726-runner-task-contract branch November 7, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants