From fb8a6a52b0c61114609a9b234d6237bf99019d42 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Wed, 22 Jul 2026 21:27:53 -0700 Subject: [PATCH 1/2] fix(fugu): accept a bare-int access index in the workflow parser (#225) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_normalize_access` validated one `access_list` entry of a Conductor workflow proposal. It accepted a scalar string digit ("0" -> [0]), None/"" (-> []), and lists of indices — but a bare scalar int (0) fell through every branch to `return None`, which rejects the *entire* workflow. So two semantically identical proposals disagreed purely on notation: access_list=[[], 0] -> parsed_ok=False, training_reward=0.0 access_list=[[], "0"] -> parses fine A bare integer is the most natural thing a model emits for "read step N", and the parser's stated goal is to avoid false negatives, so this depressed parse-rate on otherwise-valid workflows. Add a bare-int branch that returns [acc] when 0 <= acc < step_index, matching the accepted "0" / [0] forms. `bool` is rejected first since it subclasses int and is never a step index. Adds tests/test_fugu_workflow_bare_int_access.py: the three equivalent forms parse and produce an identical workflow, and the rejections that matter still hold (bool, negative, forward reference, self reference at step 0), plus the pre-existing forms are unchanged. Fixes #225 --- src/trinity/fugu/workflow.py | 15 +++- tests/test_fugu_workflow_bare_int_access.py | 85 +++++++++++++++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tests/test_fugu_workflow_bare_int_access.py diff --git a/src/trinity/fugu/workflow.py b/src/trinity/fugu/workflow.py index 6c2cd75..310b993 100644 --- a/src/trinity/fugu/workflow.py +++ b/src/trinity/fugu/workflow.py @@ -192,9 +192,9 @@ def _list_candidates(text: str, names: tuple[str, ...]) -> list[tuple[int, list] def _normalize_access(acc: object, step_index: int) -> object | None: """Validate/normalize one access entry. Returns ``None`` if invalid. - Valid forms: ``"all"``, ``[]``/``None`` (query only), or a list of integer - indices that strictly precede ``step_index`` (a forward reference is an - invalid DAG and rejects the whole workflow). + Valid forms: ``"all"``, ``[]``/``None`` (query only), a bare integer index, + or a list of integer indices that strictly precede ``step_index`` (a forward + reference is an invalid DAG and rejects the whole workflow). """ if acc is None: return [] @@ -208,6 +208,15 @@ def _normalize_access(acc: object, step_index: int) -> object | None: j = int(s) return [j] if j < step_index else None return None + if isinstance(acc, bool): + # `bool` subclasses `int`, but True/False is never a step index. + return None + if isinstance(acc, int): + # A bare int is the most natural way to say "read step N", and is + # semantically identical to the already-accepted "0" / [0] forms. + # Falling through to `return None` here rejected the whole workflow, + # depressing parse-rate (and so training reward) on valid proposals. + return [acc] if 0 <= acc < step_index else None if isinstance(acc, (list, tuple)): if len(acc) == 1 and isinstance(acc[0], str) and acc[0].strip().lower() == "all": return "all" diff --git a/tests/test_fugu_workflow_bare_int_access.py b/tests/test_fugu_workflow_bare_int_access.py new file mode 100644 index 0000000..f89252e --- /dev/null +++ b/tests/test_fugu_workflow_bare_int_access.py @@ -0,0 +1,85 @@ +"""Offline tests for bare-int access indices in the fugu workflow parser (#225). + +`_normalize_access` accepted a scalar string digit ("0" -> [0]) and lists of +indices, but a bare scalar int (0) fell through every branch to `None`, which +rejects the *whole* workflow — so a semantically identical proposal scored +`parsed_ok=False` / `training_reward=0.0` purely on notation. These tests pin +that the three equivalent forms agree, and that the rejections that matter +(bool, negative, forward reference, self reference) still hold. No GPU/network. +""" +from __future__ import annotations + +import pytest + +from trinity.fugu.workflow import _normalize_access, parse_workflow + +_BASE = 'model_id=[0,1]\nsubtasks=["solve","answer"]\naccess_list=[[], {}]' + + +# --------------------------------------------------------------------------- # +# The bug: bare int was rejected end-to-end +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize("form", ["0", '"0"', "[0]"]) +def test_equivalent_access_forms_all_parse(form): + _, parsed_ok = parse_workflow(_BASE.format(form), 3) + assert parsed_ok is True + + +def test_equivalent_access_forms_produce_identical_workflow(): + bare, _ = parse_workflow(_BASE.format("0"), 3) + string, _ = parse_workflow(_BASE.format('"0"'), 3) + listed, _ = parse_workflow(_BASE.format("[0]"), 3) + assert bare == string == listed + + +# --------------------------------------------------------------------------- # +# Helper: accepted bare ints +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize("idx", [0, 1, 2]) +def test_bare_int_normalizes_like_its_list_form(idx): + assert _normalize_access(idx, 3) == [idx] + # ...and matches the already-supported string / list spellings. + assert _normalize_access(idx, 3) == _normalize_access(str(idx), 3) + assert _normalize_access(idx, 3) == _normalize_access([idx], 3) + + +# --------------------------------------------------------------------------- # +# Guards: what must still be rejected +# --------------------------------------------------------------------------- # +@pytest.mark.parametrize("bad", [True, False]) +def test_bool_is_not_a_step_index(bad): + # bool subclasses int, so it must be rejected before the int branch. + assert _normalize_access(bad, 3) is None + + +@pytest.mark.parametrize("bad", [-1, -5]) +def test_negative_index_rejected(bad): + assert _normalize_access(bad, 3) is None + + +@pytest.mark.parametrize("bad", [3, 4, 99]) +def test_forward_reference_rejected(bad): + # An index at/after the current step is an invalid DAG edge. + assert _normalize_access(bad, 3) is None + + +def test_self_reference_at_first_step_rejected(): + assert _normalize_access(0, 0) is None + + +def test_bare_int_forward_reference_rejects_whole_workflow(): + # Step 1 referencing step 5 is still invalid, bare-int notation or not. + _, parsed_ok = parse_workflow(_BASE.format("5"), 3) + assert parsed_ok is False + + +# --------------------------------------------------------------------------- # +# Unchanged behaviour +# --------------------------------------------------------------------------- # +def test_existing_forms_unchanged(): + assert _normalize_access(None, 3) == [] + assert _normalize_access("all", 3) == "all" + assert _normalize_access("", 3) == [] + assert _normalize_access("query", 3) == [] + assert _normalize_access([0, 1], 3) == [0, 1] + assert _normalize_access("not-an-index", 3) is None From 370951c0086a5cd63db0ac40b96cb6886b9f700f Mon Sep 17 00:00:00 2001 From: minion1227 Date: Wed, 22 Jul 2026 21:53:00 -0700 Subject: [PATCH 2/2] docs(journal): log the bare-int access index rejection (#225) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md §6 requires a dated JOURNAL entry for every mistake/fix; the original commit changed workflow.py and tests but skipped the lab notebook. --- docs/JOURNAL.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index 13a8715..f1b0e19 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -18,6 +18,31 @@ protocol. **Newest entries at the top.** Tag each entry with one or more of: --- +## 2026-07-23 — workflow parser rejected a bare-int access index #mistake #fix #repro +**Context:** issue #225 — `_normalize_access` in `src/trinity/fugu/workflow.py` validates one +`access_list` entry of a Conductor workflow proposal ("which earlier steps may this step read"). +**Expected:** `access_list=[[], 0]` parses like the semantically identical `[[], "0"]` / `[[], [0]]`. +**Actual:** the two spellings disagreed purely on notation — the bare int dropped the **whole** +workflow: +```python +parse_workflow('model_id=[0,1]\nsubtasks=["solve","answer"]\naccess_list=[[], 0]', 3) # (None, False) +parse_workflow('model_id=[0,1]\nsubtasks=["solve","answer"]\naccess_list=[[], "0"]', 3) # parses fine +``` +**Root cause:** the function handled a scalar **string** digit (`"0"` -> `[0]`), `None`/`""` (-> `[]`), +and **lists** of indices, but a bare scalar **int** matched no branch and fell through to +`return None` — and `None` from `_normalize_access` rejects the entire proposal, not just that entry. +**Fix / decision:** add a bare-`int` branch returning `[acc]` when `0 <= acc < step_index`, matching +the already-accepted `"0"`/`[0]` forms, and document the form in the docstring. `bool` is rejected +**first**, since it subclasses `int` and is never a step index. A bare integer is the most natural +thing a model emits for "read step N", and the parser's stated design goal is to avoid false +negatives; rejecting it depressed parse-rate on otherwise-valid workflows, and a rejected workflow +means `parsed_ok=False`, `training_reward=0.0`, `is_correct=0`. Added +`tests/test_fugu_workflow_bare_int_access.py` (offline): the three equivalent forms produce an +identical workflow, plus guards that the rejections that matter still hold — `True`/`False` +(bool-before-int ordering), negatives, forward references (`>= step_index`), and self-reference at +step 0. +**Follow-up:** none. Note #226 proposes the same one-line fix independently. + ## 2026-07-12 — code grader: add resource limits on top of the HOME/secrets fix #security #decision **Context:** issue #71 — the code grader (`run_pass_at_1`) runs untrusted miner/LLM candidate code. The core secret-leak fix (isolated throwaway HOME/cwd, scrubbed env, `python -I`) already landed on main. **Finding:** main's sandbox closes the HOME/secrets exfiltration vector but has **no resource limits** — an untrusted candidate can still exhaust host memory or fork-bomb the eval box within its wall-clock timeout (verified: a 4 GiB `bytearray` allocation runs to completion and "passes" on main).