Resolve _send_llm_request default max_tokens binding to support runtime overrides

[Copilot]

This PR addresses the review comment about Python default argument evaluation in _send_llm_request: max_tokens was bound at function definition time, so later runtime updates to the module default were ignored. The change makes default resolution dynamic when callers omit max_tokens.

• Runtime default resolution

  • Changed _send_llm_request(max_tokens=...) from a definition-time constant to an optional parameter resolved inside the function body.
  • This ensures monkeypatched/config-updated _DEFAULT_MAX_TOKENS values are used by defaulted calls.

• Focused regression coverage

  • Added a targeted test that monkeypatches termstory.ai._DEFAULT_MAX_TOKENS and asserts the outbound request payload uses the patched value when max_tokens is not passed.

def _send_llm_request(..., max_tokens: Optional[int] = None, ...):
    if max_tokens is None:
        max_tokens = _DEFAULT_MAX_TOKENS

[git-miku]

👋 Thanks for the PR, @copilot!

To get this PR reviewed, please ⭐ star this repo — it's free and helps others discover it. Once starred, run :miku /check-star on this PR and I'll start helping out.

A do-not-merge/needs-star label has been applied to block merge until then.

[git-miku]

👋 Thanks for the PR, @copilot!

To get this PR reviewed, please ⭐ star this repo — it's free and helps others discover it. Once starred, run :miku /check-star on this PR and I'll start helping out.

A do-not-merge/needs-star label has been applied to block merge until then.

[greptile-apps]

Greptile Summary

This PR fixes the Python default argument evaluation issue in _send_llm_request where max_tokens was bound to _DEFAULT_MAX_TOKENS at function definition time, preventing runtime overrides. The fix changes the signature to max_tokens: Optional[int] = None with an in-body fallback, and adds a targeted test using monkeypatch to verify the behaviour.

• termstory/ai.py: Signature change from max_tokens: int = _DEFAULT_MAX_TOKENS to max_tokens: Optional[int] = None with a runtime resolution guard inside the function body.
• tests/test_ai.py: New test test_send_llm_request_uses_runtime_default_max_tokens monkeypatches ai._DEFAULT_MAX_TOKENS and calls _send_llm_request without passing max_tokens, asserting the patched value propagates to the outbound payload.

Confidence Score: 5/5

Safe to merge — the change is a minimal, well-targeted fix with a corresponding regression test.

The fix correctly moves default resolution from definition time to call time, the new test directly verifies the monkeypatching scenario, and no call paths are broken. The only outstanding item is a pre-existing style note about redundant explicit pass-throughs at five call sites, which was already raised in the previous review cycle.

No files require special attention.

Important Files Changed

Filename	Overview
termstory/ai.py	Signature change to _send_llm_request fixes definition-time default binding; 5 existing call sites still pass max_tokens=_DEFAULT_MAX_TOKENS explicitly (P2 style issue, already flagged in prior review).
tests/test_ai.py	Adds a focused regression test for the runtime default resolution; test is correctly structured and assertions are accurate.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Caller invokes _send_llm_request(...)"] --> B{max_tokens passed?}
    B -- "Yes (explicit value)" --> D["Use caller-supplied max_tokens"]
    B -- "No (None / omitted)" --> C["Read _DEFAULT_MAX_TOKENS\nat call time"]
    C --> D
    D --> E["Build request body\nwith resolved max_tokens"]
    E --> F["POST to LLM endpoint"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["Caller invokes _send_llm_request(...)"] --> B{max_tokens passed?}
    B -- "Yes (explicit value)" --> D["Use caller-supplied max_tokens"]
    B -- "No (None / omitted)" --> C["Read _DEFAULT_MAX_TOKENS\nat call time"]
    C --> D
    D --> E["Build request body\nwith resolved max_tokens"]
    E --> F["POST to LLM endpoint"]

Loading

_{Reviews (2): Last reviewed commit: "fix(ai): resolve max_tokens default at r..." | Re-trigger Greptile}

[bitflicker64]

@copilot fix whatever greptile flagged

[Copilot]

@copilot fix whatever greptile flagged

Addressed in ad2dcc4: the greptile flag around default binding/monkeypatch behavior is fixed by resolving max_tokens at runtime in _send_llm_request, and tests/test_ai.py now validates the runtime override path without raising=False.

[bitflicker64]

@greptile-apps review

[bitflicker64]

/lgtm /approve /merge

[git-miku]

Approved via Miku.

[git-miku]

miku /lgtm

✅ LGTM! Label added by @bitflicker64.

Use :miku /lgtm cancel to retract.

---

miku /approve

✅ Approved! 🎉

---

miku /merge

✅ Merged! 🎉