Fix reference passing for multi-round runs in bot.py (closes #30, closes #26)

[Ranoobaba]

What was broken (#30)

With --rounds 2, the first round works, but the second round sends each reference model a system prompt that ends like this:

Responses from models:
1. P
2. l
3. e
4. a
...

That is a previous answer spelled out one character at a time, which is exactly the paste in #30.

Root cause: at the end of each round, bot.py stores the collected responses as a flat list of strings:

references = [item["output"] for item in eval_set]
data["references"] = references
eval_set = datasets.Dataset.from_dict(data)

Dataset.from_dict splits every column across rows, so in the next round each row's references field is a single string (that model's own previous answer). generate_with_references only checks len(references) > 0, which is truthy for a non empty string, so inject_references_to_messages runs for i, reference in enumerate(references) over a string and enumerates its characters into the "Responses from models:" list.

There are two defects folded together:

1. Type. Each row carries a str where a list[str] is required, which produces the visible character splitting.
2. Semantics. Even wrapped as a one element list, each model would still only see its own previous answer. The MoA method (arXiv:2406.04692, Section 2.2, equation 1) requires every agent in a layer to receive all n outputs of the previous layer, plus the original prompt. This is also what the repo's own eval scripts do: generate_for_alpaca_eval.py and generate_for_mt_bench.py pass the full prev_references list to every proposer each round.

The fix

Store one full copy of the response list per dataset row, so each model in a later round receives all n responses from the round before it:

data["references"] = [list(references) for _ in range(len(reference_models))]

A fresh list per row rather than one list repeated, matching how the instruction column two lines above is built.

Models that failed are now skipped instead of passed on. generate_together returns None when a model errors out, and both eval scripts already drop those with if reference is not None. Without this, the fix would put the text None into the system prompt of every other model, silently. bot.py prints a line saying how many were dropped, since it is interactive and a missing model changes the answer.

The references column is also initialised and reset as empty lists rather than empty strings. Empty lists skip injection exactly as empty strings did (len(...) == 0), so round 1 is untouched. This is type hygiene, not part of the behaviour fix.

Three decisions, each made deliberately:

1. Fix at the data construction site, not inside process_fn. PR fix(bot): fix a bug where references was a string instead of an array #18 (credit to @gligoran for first spotting the string versus list mismatch) wraps the string into a list inside process_fn. That stops the character splitting but leaves the second defect in place: each model still only sees its own previous answer, which is not the MoA architecture. Fixing where data["references"] is built restores the full fan out.
2. The same complete reference set for every agent, rather than a per row slice. That is what the paper and the eval scripts do.
3. No change to round 1, the aggregator call, or any prompt text. The aggregator already received the correct flat list, because it bypasses the dataset round trip, which is why only rounds of 2 or more were broken.

Why the system prompt, not the user prompt (#26)

#26 asked why moa.py joined the proposers' answers into the user message while inject_references_to_messages puts them into the system message. That difference was real when #26 was filed, because moa.py then did {"role": "user", "content": ",".join(results)}, and it was unintentional demo drift rather than a design decision. #47 (commit 0bb4bc8) already moved moa.py to the system prompt placement, and advanced-moa.py uses it too, so the inconsistency is already gone from main. The paper does not pin the chat role of the Table 1 "Aggregate and Synthesize" prompt, so the authoritative reference is the implementation the paper's results were produced with, namely inject_references_to_messages together with the eval scripts. That means the references go in the system prompt while the original user prompt stays in the user turn, which is the + x_1 term of equation 1 in Section 2.2. This PR records that answer in a docstring on inject_references_to_messages, including the list not string contract that #30 tripped over, so the question is answered in the code rather than in a closed issue. No behaviour changes in utils.py.

Verification

The repo has a tests.py, but it calls the live Together and OpenAI APIs and asserts exact model output strings, so it cannot run offline or in CI. There was no offline check of the round loop at all, which is part of why this stayed unnoticed. This PR adds verify_main.py.

It calls the real bot.main() rather than re-implementing the round loop, so a regression fails the check regardless of how the code is written. Four things are replaced, each named in the file's docstring: utils.requests.post (captures the exact payload each proposer would receive, returns a canned answer), bot.generate_together_stream (records the aggregator payload), bot.Prompt.ask (feeds scripted answers, ending with exit), and the num_proc argument to Dataset.map (bot.py maps with one worker per model, and spawned workers would not inherit the patched requests.post). Outbound sockets raise, as a hard stop in case any of that is bypassed. No API key is needed and no request leaves the machine.

Six scenarios: rounds=1, rounds=2, rounds=3, rounds=2 across two chat turns, rounds=2 with multi_turn off, and rounds=2 with one model failing.

On current main, rounds=1 passes and the four multi round scenarios fail:

PASS [rounds=1 (default path stays untouched)]
FAIL [rounds=2 (issue #30)]
       model-A: saw 35 references, expected 4
       model-A: references split into characters, first few ['<', 'm', 'o', 'd', 'e', 'l']

The failing model scenario does not report at all on main, it raises first:

    if len(references) > 0:
TypeError: object of type 'NoneType' has no len()

After the fix all six pass. Because the check asserts behaviour rather than matching source text, it also catches a weakened fix: forwarding only the first model's answer ([:1]) fails four scenarios with saw 1 references, expected 4.

rounds=1 passing on both sides is the guard that the default path is untouched.

Known limitations

1. Each proposer in a later round now receives all n previous answers instead of one, so the reference text sent per round grows with the square of the number of reference models. It does not grow with the number of rounds, because references are replaced each round rather than appended, and the number of requests is unchanged. With the default four models this is small, but it is worth knowing before combining a long reference list with a high --rounds.
2. bot.py's round loop is still a separate copy of the eval scripts' prev_references loop rather than shared code. Unifying them felt like too much churn for a bugfix.
3. --rounds 0 raises UnboundLocalError. That is pre existing on main and untouched here, since it is a separate bug from the one --rounds 2 seems broken #30 reports.
4. multi_turn is changed by this diff and is covered by the added check, but it is not reachable from the command line: it has no type annotation, so typer exposes it as a string and --multi-turn False passes the truthy string "False". Also pre existing and left alone.
5. The pre existing blank line issue at the top of utils.py that current black flags is left untouched to keep the diff minimal. The lines added here are black clean.
6. verify_main.py is a standalone script at the repo root, because the repo has no test runner to hook into. Happy to move or reshape it if you would rather it lived elsewhere.

Closes #30. Closes #26. Related work: this supersedes the approach in #18 (see above), and #48 covers the same Section 2.2 residual connection from the diagram side.

[broly-code-security-scanner]

Broly Security Scan

Note

Baseline snapshot is missing for this repo. Broly is running in PR-only fallback mode until the first scheduled baseline completes. This does not block the PR.

Note

✅ Clean scan
No vulnerabilities detected in this PR.

Note

Re-scan this PR anytime with /broly scan — useful after /broly undismiss, or to refresh findings without a new push.

Broly — SAST (zai-org/GLM-5.2) · Secrets · SCA · IaC · GH Actions · Base Images · Supply Chain Threats · Exploit Chains · Adversarial Verification

We're continuously improving Broly's accuracy and finding quality — your feedback is valuable. False positives, missed findings, bugs, and feature requests all welcome.

 

[Ranoobaba]

@Nutlope this is ready for a look when you have time. It fixes the character splitting in #30, where the round loop stored references as a flat list so Dataset.from_dict handed each model a single string. It also answers #26 in a docstring. It adds verify_main.py, an offline check that drives the real bot.main() and needs no API key, since the repo has no offline test today. Happy to split the docstring out if you would rather keep the bugfix alone.