[XXXXXQ-0206]

ChainResponse defaults chain_limit to 10, but no public Python entry point applied it: Conversation.chain_limit was None and Model.conversation() passed chain_limit=None, so model.chain() and model.conversation().chain() ran with no limit. The CLI is bounded separately because --chain-limit defaults to 5.

class LoopingModel(llm.Model):
    model_id = 'looping'
    supports_tools = True

    def execute(self, prompt, stream, response, conversation):
        response.add_tool_call(llm.ToolCall(name='ping', arguments={}))
        yield 'calling ping'

for response in LoopingModel().chain('go', tools=[ping]).responses():
    response.text()   # before: never stops; after: ValueError: Chain limit of 10 exceeded.

The default now lives in one place, models.DEFAULT_CHAIN_LIMIT, used by both _BaseConversation.chain_limit and ChainResponse, and the two conversation() factories default to it instead of None, so the existing self.chain_limit fallbacks have a value to fall back to. An explicit chain_limit still wins.

-    chain_limit: int | None = None          # _BaseConversation
+    chain_limit: int | None = DEFAULT_CHAIN_LIMIT

-        chain_limit: int | None = None      # Model.conversation / AsyncModel.conversation
+        chain_limit: int | None = DEFAULT_CHAIN_LIMIT,

tests/test_chain_limit.py covers the default, both unbounded entry points, and that an explicit limit still wins. The two chain tests fail on the current code — they never finish — and pass with the change; test_chain_callbacks.py and test_pause_resume.py are 20 passed before and after.

Not run: the full suite needs the llm-echo plugin and network-marked tests and did not finish here; I verified with the files above.

Fixes #1773

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[chrikrah]

@XXXXXQ-0206, the code does what #1773 describes, and whether it should land is Simon's call, not mine. The four new tests pass on your head 68b5227. With models.py from your parent 2465dfc the first test fails and the file hangs until timeout kills it.

# head 68b52275, exported with git archive, llm imported from that tree; Python 3.12.3, pytest 9.1.1
$ python -m pytest tests/test_chain_limit.py -q -p no:cacheprovider
4 passed in 0.35s

# same tree, llm/models.py from 2465dfc
$ timeout 15 python -m pytest tests/test_chain_limit.py -q -p no:cacheprovider
F
rc=124

$ python probe.py     # .chain_limit of the ChainResponse each call builds, echo model; one run per tree, side by side
                                          2465dfc   68b5227
model.chain()                             None      10
conversation().chain()                    None      10
conversation().chain(chain_limit=None)    None      10
conversation(chain_limit=None).chain()    None      None
conversation(chain_limit=0).chain()       0         0

$ git merge-tree --write-tree origin/main 68b5227 >/dev/null; echo $?    # main aebaca2
0
# not run: the full suite; the network-marked tests

The case for a bound: before 2df619e, Conversation.chain() passed no limit, so the ChainResponse default of 10 applied. The case against: docs/python-api.md:138 calls model.chain() a loop that "keeps going until the model stops requesting tools", and this PR leaves that line alone. The cost lands on Python callers. A loop past 10 tool calls, or a deliberate .chain(chain_limit=None), now raises ValueError: Chain limit of 10 exceeded. with no deprecation step, so this belongs in the changelog.

@simonw, was the unbounded Python chain after 2df619e intended? If not, python-api.md:138 needs the same change, plus a line naming 0 as the opt-out.

[XXXXXQ-0206]

Thanks @chrikrah — picked up all three points in f8bb8a6:

• docs/python-api.md:138 now states the bound instead of describing the loop as running until the model stops requesting tools, and names chain_limit=0 as the opt-out;
• added a short chain_limit=0 example;
• added a changelog entry under Unreleased, flagged as a behaviour change since ValueError is now raised where it previously was not.

I also added test_chain_limit_zero_opts_out so the documented opt-out is covered — chain_limit=0 is falsy in the if self.chain_limit and count >= self.chain_limit guard, so the loop keeps going, which is what the docs now say. tests/test_chain_limit.py is 5 passed on the new head.

On the full suite: I cannot get it green locally, but the failures are pre-existing in this environment rather than from this PR. tests/test_templates.py gives 15 failed, 31 passed, 15 errors on this branch and the identical 15 failed, 31 passed, 15 errors on unmodified aebaca2; the failure is httpx2.InvalidURL: Invalid port: ':1]' when the template tests parse the local IPv6 URL. I have not verified the rest of the suite the same way, so I am not claiming it is green.

git merge-tree --write-tree origin/main f8bb8a6 exits 0, so the docs commits do not conflict with current main (which has moved past aebaca2).

[chrikrah]

@XXXXXQ-0206 the docs, the changelog line and the chain_limit=0 test cover my three points at f8bb8a6. One thing will fail the Test workflow once it runs, and I missed it at 68b5227 too, so my approval holds only once black passes.

blocking: test.yml runs black --check ., and it wants to reformat tests/test_chain_limit.py.

$ python -m pytest tests/test_chain_limit.py -q -p no:cacheprovider   # f8bb8a6, llm imported from the export, Python 3.12
5 passed in 0.15s
$ black --check .   # black 26.10.1
would reformat tests/test_chain_limit.py
1 file would be reformatted, 56 files would be left unchanged.
$ black --check .   # base 2465dfc
56 files would be left unchanged.

Can you push black tests/test_chain_limit.py? With that in, this is ready for Simon.