refactor: extract FlowRunner, fix /login warning, harden wire/deps - #84
Merged
Conversation
Introduces models_dev.py: fetches models.dev/api.json, caches to disk with
24h TTL, flattens provider→model hierarchy into {model_id: ModelPrice} with
canonical provider priority and versioned-id filtering. Atomic write prevents
partial-file reads on concurrent access.
…o hardcoded table
…c context _maybe_print_cost_panel was deconstructing the Panel and printing title/renderable separately, discarding the ROUNDED border and border_style. Fix by making the function async and passing the Panel object directly to console.print. Also offloads the synchronous disk I/O in _load_cost_stats to asyncio.to_thread, matching the pattern already used for load_all_stats in the /stats handler. Tests updated to await the async function and assert a Panel instance with the correct title is passed to console.print.
…isolation in editor test
Registers AlibabaAdapter in the ADAPTERS dict so /usage no longer falls into the "not yet available" branch for managed:alibaba providers. The adapter probes the DashScope /api/v1/quotas endpoint and falls back to the process-wide rate-limit cache populated from completion headers.
Move FlowRunner and the flow constants out of the 2209-line PythinkerSoul module into soul/flow_runner.py. FlowRunner collaborates with the soul only through its public turn machinery, so the dependency is type-only (TYPE_CHECKING); FLOW_COMMAND_PREFIX is re-exported from pythinkersoul to preserve the shell UI's import path. No behavior change. First step of the staged PythinkerSoul (Phase A) decomposition.
Setting Application.min_redraw_interval activated prompt_toolkit's coroutine-based redraw throttle (async def redraw_in_future). During the /login prompt-app/loop handoff that coroutine could be created then dropped un-awaited, emitting a noisy RuntimeWarning. Switch to max_render_postpone_time, which throttles redraws via a coroutine-free path, so the warning is impossible by construction.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/models_dev.py`:
- Around line 57-77: The parsing loop in _flatten_catalog()/load_catalog()
assumes cost fields are float-coercible and can raise on malformed upstream
payloads; update the loop that constructs ModelPrice (in the for provider_id...
and for model_id... blocks) to defensively validate and coerce each cost
subfield (input, output, cache_read, cache_write) inside a small helper or
inline try/except: if a value is missing, non-numeric, or fails float()
conversion, treat it as 0.0 or skip that model entry instead of letting
exceptions propagate; ensure you use the same target selection (canonical vs
fallback) and keep ModelPrice creation only after successful safe coercion so
load_catalog() returns safely on bad payloads.
In `@src/pythinker_code/soul/flow_runner.py`:
- Around line 196-203: The wire stream may miss a TurnEnd if soul._turn raises
in _flow_turn; modify the _flow_turn function to call wire_send(TurnBegin(...))
before awaiting soul._turn and ensure wire_send(TurnEnd()) is executed in a
finally block so TurnEnd is always emitted even on exceptions; keep the await
res = await soul._turn(...) (or rethrow its exception) and return the
TurnOutcome as before, referencing _flow_turn, wire_send, TurnBegin, TurnEnd,
and soul._turn to locate the change.
- Around line 135-165: The retry loop in _flow_turn inside FlowRunner (around
_flow_turn, _execute_flow_node, moves and steps_used) can loop indefinitely on
invalid decision choices; ensure the moves/step cap is enforced on each LLM turn
by incrementing the move/step counter (or a separate retry counter) immediately
after each _flow_turn call and before re-prompting, and check it against the max
moves/steps limit (the same limit used after _execute_flow_node) so that if the
cap is exceeded you return None (or the appropriate stop) instead of retrying
forever; update the logic around _flow_turn, steps_used, and _match_flow_edge to
bail out when the cap is reached.
In `@src/pythinker_code/tools/file/grep_local.py`:
- Around line 326-343: The cached _resolved_rg_path may point to a removed or
non-executable binary and is returned unconditionally; update the resolver to
revalidate the cached path before returning by checking that
Path(_resolved_rg_path).exists() and is executable (or validates via
_find_existing_rg) and if that check fails, clear/ignore _resolved_rg_path and
proceed to run the existing discovery logic (_rg_binary_name, _find_existing_rg)
and the async download path (_RG_DOWNLOAD_LOCK, _download_and_install_rg) so
discovery/download will run again; ensure you update all early-return branches
to set _resolved_rg_path only after a successful validation.
- Around line 178-188: _is_runnable currently blocks using subprocess.run and
may stall the event loop when called from async paths (via _ensure_rg_path and
Grep.__call__); make it non-blocking by converting it to an async helper that
uses asyncio.create_subprocess_exec (or runs the check in an executor via
asyncio.to_thread) and enforce the timeout with asyncio.wait_for, then update
_ensure_rg_path (and call sites such as Grep.__call__) to await the new async
_is_runnable; keep function name _is_runnable to minimize changes and preserve
return semantics (bool) and handle/translate OSError and TimeoutError to return
False as before.
In `@src/pythinker_code/ui/shell/usage_adapters/alibaba.py`:
- Around line 57-58: The current parsing uses "or" to set total and used from
payload which treats valid zeros as falsy and discards them; update the
assignments for total and used to prefer token_quota/token_used when those keys
are present (check for payload.get("token_quota") is not None or use
"token_quota" in payload) and fall back to total_quota/total_used otherwise so
that 0 values are preserved; locate the code that sets total and used from
payload and replace the boolean-or logic with explicit None/key-presence checks.
In `@src/pythinker_code/ui/shell/usage.py`:
- Around line 245-253: The function _maybe_print_cost_panel currently swallows
all exceptions; update its except block to log the failure at debug level rather
than silently dropping it: catch Exception as e and call the module logger
(e.g., logger.debug) with a short message like "cost panel failed to render" and
include the exception details (use exc_info=True or format the exception) so the
function still returns without raising; reference _maybe_print_cost_panel,
_load_cost_stats and _build_cost_panel when locating the change.
In `@tasks/decomposition-plan.md`:
- Around line 65-66: Add a blank line immediately after the Markdown heading "##
Out of scope (logged, not fixed here)" so the following list (the dash item
referencing `models_dev.py:65`) is separated by an empty line; this will satisfy
markdownlint rule MD022. Locate the heading text in tasks/decomposition-plan.md
and insert a single empty line before the list item that starts with "-
`models_dev.py:65`".
In `@tests/test_models_dev.py`:
- Around line 151-156: Replace the brittle internal-method patching of
models_dev._do_fetch (and the fake_fetch helper) with a mock of the external I/O
entrypoint new_client_session so tests exercise observable behavior of
refresh_catalog; specifically, in tests around refresh_catalog (and the similar
block at 177-182) stop patching _do_fetch and instead patch
models_dev.new_client_session to return a controllable session/response
(simulate success or failure), then assert observable outcomes such as whether a
network fetch occurred, whether the cache file/timestamp changed, or the return
value of refresh_catalog.
- Around line 24-65: Add a regression test for _flatten_catalog that covers
malformed cost values: extend tests/test_models_dev.py with a new test (e.g.,
test_flatten_malformed_costs_skipped) that uses _fixture_dict()/a modified
fixture to include a model entry whose "input" cost is a non-numeric value
(e.g., "n/a" or an object) and then assert that _flatten_catalog either skips
that model key from the result or returns a safe default numeric cost (e.g.,
0.0) depending on the intended behavior; reference the parser function
_flatten_catalog and the result map keys (like "some-model" or a new unique
model id) to locate where to check for the absence or defaulted value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4204880a-d5e9-4ffc-9374-a43f83084229
📒 Files selected for processing (21)
CHANGELOG.mdpyproject.tomlsrc/pythinker_code/models_dev.pysrc/pythinker_code/soul/flow_runner.pysrc/pythinker_code/soul/pythinkersoul.pysrc/pythinker_code/tools/file/grep_local.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/stats.pysrc/pythinker_code/ui/shell/stats_pricing.pysrc/pythinker_code/ui/shell/usage.pysrc/pythinker_code/ui/shell/usage_adapters/__init__.pysrc/pythinker_code/ui/shell/usage_adapters/alibaba.pytasks/decomposition-plan.mdtests/core/test_wire_file_compat.pytests/fixtures/models-dev-subset.jsontests/test_models_dev.pytests/ui/test_usage_cost_panel.pytests/ui/usage_adapters/test_alibaba_adapter.pytests/ui_and_conv/test_redraw_throttle.pytests/ui_and_conv/test_stats_pricing.pytests/utils/test_editor.py
- models_dev: add _coerce_cost() helper; skip models with non-numeric cost fields instead of letting ValueError propagate from _flatten_catalog - flow_runner: emit TurnEnd in a finally block so wire stream stays consistent even when soul._turn raises; cap invalid-choice retries at MAX_INVALID_CHOICE_RETRIES=3 (was unbounded, dangerous with ralph loop) - grep_local: make _is_runnable async (asyncio.create_subprocess_exec), make _find_existing_rg async; validate cached _resolved_rg_path with exists()+access() before returning to handle removed binaries; drop now-unused subprocess import - alibaba: replace boolean-or fallback with key-presence check so zero token_quota/token_used values are preserved rather than discarded - usage: log at debug level instead of silently swallowing exceptions in _maybe_print_cost_panel - decomposition-plan.md: blank line after heading (MD022) - tests: add test_flatten_malformed_costs_skipped regression test; update test_grep.py callers to await async _find_existing_rg; remove unused imports in test_usage_cost_panel.py; fix ruff I001/F401 across test_models_dev.py and test_usage_cost_panel.py
- ruff format: reformat models_dev.py, grep_local.py, alibaba.py, tests/test_models_dev.py (was failing check CI) - models_dev._coerce_cost: rename parameter value→raw (prohibited name) - alibaba.AlibabaAdapter.fetch: replace bare except Exception: pass with logger.debug(..., exc_info=True) for local-stats block; add missing logger import
elkaix
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pythinker_code/models_dev.py (1)
80-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard top-level JSON shape as well.
Lines 80-88 only harden individual model entries. If the cache contains valid JSON that is not an object (for example
[]),load_catalog()still passes it to_flatten_catalog()and blows up onraw.items(), so the{}fallback contract is still broken.Proposed fix
def load_catalog() -> dict[str, ModelPrice]: @@ try: raw = json.loads(cache_path.read_text(encoding="utf-8")) except Exception: return {} + if not isinstance(raw, dict): + return {} result = _flatten_catalog(raw)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pythinker_code/models_dev.py` around lines 80 - 88, The code only guards individual model entries when building ModelPrice but doesn't validate the top-level JSON shape; update load_catalog() (and/or _flatten_catalog()) to ensure the loaded JSON 'raw' is a mapping/dict before iterating (e.g., check isinstance(raw, dict) and if not, treat as empty dict or raise a controlled error), so that subsequent calls to raw.items() are safe; keep the existing per-entry try/except around ModelPrice/_coerce_cost but add the top-level guard to enforce the {} fallback contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/tools/file/grep_local.py`:
- Around line 331-335: The cache validation currently uses Path.exists() which
also returns true for directories; change the check in the function that handles
_resolved_rg_path to verify that the resolved path is a regular file and
executable by replacing the p.exists() check with p.is_file() (keep the
os.access(..., os.X_OK) check) so we only accept executable files, not
directories; update the logic around Path(_resolved_rg_path) / p to use
is_file() for the semantic intent.
---
Duplicate comments:
In `@src/pythinker_code/models_dev.py`:
- Around line 80-88: The code only guards individual model entries when building
ModelPrice but doesn't validate the top-level JSON shape; update load_catalog()
(and/or _flatten_catalog()) to ensure the loaded JSON 'raw' is a mapping/dict
before iterating (e.g., check isinstance(raw, dict) and if not, treat as empty
dict or raise a controlled error), so that subsequent calls to raw.items() are
safe; keep the existing per-entry try/except around ModelPrice/_coerce_cost but
add the top-level guard to enforce the {} fallback contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c9f973af-b649-4e9e-8af0-4a42040ddb6b
📒 Files selected for processing (9)
src/pythinker_code/models_dev.pysrc/pythinker_code/soul/flow_runner.pysrc/pythinker_code/tools/file/grep_local.pysrc/pythinker_code/ui/shell/usage.pysrc/pythinker_code/ui/shell/usage_adapters/alibaba.pytasks/decomposition-plan.mdtests/test_models_dev.pytests/tools/test_grep.pytests/ui/test_usage_cost_panel.py
…nt_session - ruff format: test_alibaba_adapter.py and test_stats_pricing.py reformatted (multi-context manager style + line length — clears the only failing CI check) - test_models_dev: replace patch.object(_do_fetch) with mock of new_client_session in noop-within-TTL and fetches-when-stale tests; exercises real _do_fetch path instead of stubbing internal implementation detail - models_dev: update _do_fetch docstring (no longer patched in tests)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pythinker_code/ui/shell/usage_adapters/alibaba.py (1)
133-134:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not silently swallow quota fetch failures.
except (aiohttp.ClientError, TimeoutError): passhides real fetch failures and can fall through to the generic “No usage recorded yet” note, which is misleading on transient outages. Log at debug level and add a failure note for this branch.As per coding guidelines, “Flag exception handlers that silently swallow errors without logging or re-raising” and “Keep exceptions actionable. User-facing CLI errors should explain what to do next.”
Suggested patch
- except (aiohttp.ClientError, TimeoutError): - pass + except (aiohttp.ClientError, TimeoutError) as e: + logger.debug("quota API request failed: {error}", error=e, exc_info=True) + notes.append("DashScope quota API unavailable right now. Retry in a moment.")Also applies to: 160-161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pythinker_code/ui/shell/usage_adapters/alibaba.py` around lines 133 - 134, Replace the silent except (aiohttp.ClientError, TimeoutError): pass in the Alibaba quota-fetch routine with a debug-level log of the exception (including the caught exception object) and add/attach a user-visible failure note to the usage result (e.g., append a “Failed to fetch quota” message to the usage/notes field or set a failure flag in the usage dict) so the CLI shows a transient outage message rather than “No usage recorded yet”; do the same change for the second identical except block later in the file (both blocks that currently catch aiohttp.ClientError and TimeoutError).Source: Coding guidelines
♻️ Duplicate comments (1)
tests/test_models_dev.py (1)
170-191: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winStop patching private
_do_fetchin refresh tests.Line 186 and Line 214 patch an internal method, which makes these tests brittle against harmless refactors. Mock
new_client_sessionand assert observable outcomes (fetch called/not called, cache file updated) instead.Proposed test adjustment
@@ - fetch_called = [] - - async def fake_fetch(path): - fetch_called.append(True) - return False - - with patch.object(models_dev, "_do_fetch", fake_fetch): + with patch("pythinker_code.models_dev.new_client_session") as mock_session_factory: result = await models_dev.refresh_catalog(force=False) @@ - assert len(fetch_called) == 0 + assert mock_session_factory.call_count == 0 @@ - fetch_called = [] - - async def fake_fetch(path): - fetch_called.append(True) - return True - - with patch.object(models_dev, "_do_fetch", fake_fetch): + mock_session = AsyncMock() + mock_resp = AsyncMock() + mock_resp.text = AsyncMock(return_value=_fixture_json()) + mock_resp.__aenter__ = AsyncMock(return_value=mock_resp) + mock_resp.__aexit__ = AsyncMock(return_value=False) + mock_session.get = lambda *a, **kw: mock_resp + mock_session.__aenter__ = AsyncMock(return_value=mock_session) + mock_session.__aexit__ = AsyncMock(return_value=False) + + with patch("pythinker_code.models_dev.new_client_session", return_value=mock_session): result = await models_dev.refresh_catalog(force=False) @@ - assert len(fetch_called) == 1 + assert cache_file.exists()As per coding guidelines,
tests/**/*.py: “Flag tests that mock internal implementation details rather than observable behavior.”Also applies to: 194-218
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_models_dev.py` around lines 170 - 191, The tests currently patch the private helper _do_fetch in refresh_catalog which couples tests to internals; update the tests to mock the observable boundary new_client_session instead and assert behavior via public effects: replace patch.object(models_dev, "_do_fetch", ...) with a mock of models_dev.new_client_session that records whether an HTTP fetch would occur, then call models_dev.refresh_catalog(force=False) and assert the mock was not invoked and the cache file remained unchanged (and for the force=True case assert the mock was invoked and the cache file updated); reference the functions refresh_catalog, _do_fetch (to remove), and new_client_session when making the change.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/ui/shell/usage_adapters/alibaba.py`:
- Around line 143-145: The "Requests" row currently passes
snap.requests_remaining into the used field (label="Requests",
used=snap.requests_remaining, limit=snap.requests_limit) which mislabels
remaining as consumed; fix by either renaming the label to "Requests remaining"
or computing consumed requests with used = snap.requests_limit -
snap.requests_remaining (keep limit=snap.requests_limit). Update the call site
where label/used/limit are set (the row creation using snap.requests_remaining
and snap.requests_limit) so the semantics and label match.
---
Outside diff comments:
In `@src/pythinker_code/ui/shell/usage_adapters/alibaba.py`:
- Around line 133-134: Replace the silent except (aiohttp.ClientError,
TimeoutError): pass in the Alibaba quota-fetch routine with a debug-level log of
the exception (including the caught exception object) and add/attach a
user-visible failure note to the usage result (e.g., append a “Failed to fetch
quota” message to the usage/notes field or set a failure flag in the usage dict)
so the CLI shows a transient outage message rather than “No usage recorded yet”;
do the same change for the second identical except block later in the file (both
blocks that currently catch aiohttp.ClientError and TimeoutError).
---
Duplicate comments:
In `@tests/test_models_dev.py`:
- Around line 170-191: The tests currently patch the private helper _do_fetch in
refresh_catalog which couples tests to internals; update the tests to mock the
observable boundary new_client_session instead and assert behavior via public
effects: replace patch.object(models_dev, "_do_fetch", ...) with a mock of
models_dev.new_client_session that records whether an HTTP fetch would occur,
then call models_dev.refresh_catalog(force=False) and assert the mock was not
invoked and the cache file remained unchanged (and for the force=True case
assert the mock was invoked and the cache file updated); reference the functions
refresh_catalog, _do_fetch (to remove), and new_client_session when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dda4a095-cd25-43fb-a4c7-a73947bac691
📒 Files selected for processing (4)
src/pythinker_code/models_dev.pysrc/pythinker_code/tools/file/grep_local.pysrc/pythinker_code/ui/shell/usage_adapters/alibaba.pytests/test_models_dev.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/pythinker_code/models_dev.py (1)
121-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
load_catalog()against non-object JSON payloads.A valid-but-non-dict cache payload (for example
[]) will raise in_flatten_catalog(raw)and break the safe fallback contract. Return{}unless the parsed JSON root is a dict.Proposed fix
- try: - raw = json.loads(cache_path.read_text(encoding="utf-8")) + try: + raw = json.loads(cache_path.read_text(encoding="utf-8")) except Exception: return {} + if not isinstance(raw, dict): + return {} + result = _flatten_catalog(raw)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pythinker_code/models_dev.py` around lines 121 - 127, The parsed JSON `raw` must be validated as a mapping before calling `_flatten_catalog(raw)` and updating the cache; modify `load_catalog()` so that after `raw = json.loads(...)` you check `isinstance(raw, dict)` (or similar mapping test) and return `{}` immediately if it's not a dict, ensuring you do not call `_flatten_catalog` or set `_catalog_cache["entry"]` when the payload is an array or other non-object JSON; keep `mtime_ns` and cache update behavior only for valid dict payloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/pythinker_code/models_dev.py`:
- Around line 121-127: The parsed JSON `raw` must be validated as a mapping
before calling `_flatten_catalog(raw)` and updating the cache; modify
`load_catalog()` so that after `raw = json.loads(...)` you check
`isinstance(raw, dict)` (or similar mapping test) and return `{}` immediately if
it's not a dict, ensuring you do not call `_flatten_catalog` or set
`_catalog_cache["entry"]` when the payload is an array or other non-object JSON;
keep `mtime_ns` and cache update behavior only for valid dict payloads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e7f3eab5-07bf-45cb-ab75-1559b4b224ea
📒 Files selected for processing (4)
src/pythinker_code/models_dev.pytests/test_models_dev.pytests/ui/usage_adapters/test_alibaba_adapter.pytests/ui_and_conv/test_stats_pricing.py
- models_dev.load_catalog: guard against non-object JSON payloads so a
top-level list no longer escapes the {} fallback via _flatten_catalog
- alibaba adapter: log at debug and surface an actionable note on a
quota-fetch failure instead of silently falling through to the
misleading "No usage recorded yet"
- alibaba adapter: relabel the rate-limit "Requests" row as
"Requests remaining" to match the value it reports
Add regression tests for each.
The `check` job (make check-pythinker-code) has been red on this branch because pyright reported 36 errors across the new usage/pricing feature code. These are type-only fixes — no runtime behavior changes. - models_dev / alibaba: cast untyped JSON (.get/.items results) to dict[str, Any] at the parse boundary, matching the deepseek adapter idiom; narrow _coerce_cost with isinstance instead of float(object) - stats / usage: rename _fmt_cost -> fmt_cost; it was a private symbol imported across modules (reportPrivateUsage) - test_redraw_throttle: type the spy coroutine param so .close() resolves
This was referenced Jun 9, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidated branch with several independent changes (each is its own commit so they can be reviewed — or split — separately).
Bug fix
redraw_in_futurewarning —Application.min_redraw_intervalactivated prompt_toolkit's coroutine-based redraw throttle; during the/loginprompt-app/loop handoff that coroutine could be created then dropped un-awaited, emitting a noisyRuntimeWarning. Switched tomax_render_postpone_time(coroutine-free path), so the warning is impossible by construction. Regression test exercises the prompt_toolkit boundary directly (tests/ui_and_conv/test_redraw_throttle.py).Refactor (Phase A1 of the PythinkerSoul decomposition)
FlowRunner+ flow constants out of the 2209-linepythinkersoul.pyintosoul/flow_runner.py. Type-only dependency on the soul (TYPE_CHECKING);FLOW_COMMAND_PREFIXre-exported to preserve the shell UI import path. No behavior change.tasks/decomposition-plan.md) for the remaining A2–A7 seams.Hardening / tests
WireFilelegacy-version branch that was previously untested.dependabot.yml.Also included (pre-existing local work)
Because
origin/mainwas behind localmain, this branch also carries the usage/alibaba and models.dev pricing commits already on localmain. They are unrelated to the above and can be retargeted if a smaller PR is preferred.Verification
FlowRunner/FLOW_COMMAND_PREFIXresolve)tests/ui_and_convsuite green under-W error::RuntimeWarningSummary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests