From 1e064d515a4c1a844968eae13c5de6f362262102 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 10 Jun 2026 20:00:46 -0400 Subject: [PATCH 1/9] chore: clean completed tasks from todo.md --- tasks/todo.md | 682 +++----------------------------------------------- 1 file changed, 40 insertions(+), 642 deletions(-) diff --git a/tasks/todo.md b/tasks/todo.md index c175bece..e951477b 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,164 +1,3 @@ -# Task: Windows web 404 + PowerShell banner rendering (2026-06-10) - -## Diagnosis (verified) - -1. **`GET /?token=…` → 404 on Windows.** `src/pythinker_code/web/static/` and - `vis/static/` are gitignored build artifacts (`.gitignore:59`); only two - `web/static/brand/` files are tracked. `windows-installer.yml` and - `linux-installer.yml` freeze with PyInstaller **without building the web/vis - frontends** (unlike `release-pythinker-cli.yml`, which sets up Node and runs - the builds — the published 0.40.0 PyPI wheel contains all 448 web/static - files, so pip installs are fine). `collect_data_files()` silently collects - only the brand files → frozen app's `STATIC_DIR.exists()` is True but - `index.html` is missing → `StaticFiles(html=True)` 404s on `/`. -2. **Banner garbled in PowerShell.** `print_banner()` (utils/server.py) raw- - prints Unicode block/box art. Verified it cannot encode to cp1252 → - UnicodeEncodeError on redirected Windows stdout, garbling in legacy - consoles. Existing `ascii_glyphs_enabled()` fallback infra is bypassed. - (Alignment itself is correct — all banner lines are exactly equal width; - the "long line" in the report was a paste artifact.) - -## Plan - -- [x] utils/server.py: ASCII fallback translation (1:1 width-preserving) wired - to `ascii_glyphs_enabled()` + crash-proof printing on UnicodeEncodeError. - → verified: 6 new tests in tests/utils/test_server.py + live cp1252 run -- [x] web/app.py + vis/app.py: gate mount on `index.html`, serve explanatory - 503 page instead of bare 404 when assets missing. - → verified: tests/web/test_web_ui_assets.py (web + vis) -- [x] utils/pyinstaller.py helper + root pythinker.spec + both installer specs: - fail the freeze loudly when web/vis bundles are missing. - → verified: 2 tests in tests/utils/test_pyinstaller_utils.py + default-path run -- [x] windows-installer.yml + linux-installer.yml: set up Node and build - web/vis bundles before PyInstaller (mirrors release-pythinker-cli.yml). - → verified: YAML parses; spec syntax parses; spec guard enforces assets -- [x] CHANGELOG.md Unreleased entry. -- [x] Run ruff + pyright + targeted pytest. -- [x] Bonus bug found during run: `print_banner` crashed with TypeError on an - all-`
` banner (`max(60, *[])`); fixed + regression test. - -## Review - -- Verification: `ruff check src tests` clean, `ruff format --check` clean, - `pyright` 0 errors on all touched src files (strict mode), typos clean, - 428 passed / 1 skipped across tests/utils + tests/web + tests/vis. -- Live repro of the user scenario: `PYTHONIOENCODING=cp1252` run now prints a - perfectly aligned ASCII banner instead of raising UnicodeEncodeError. -- clean-code-guard pass: no violations; one documented exception — the - required-UI-assets check is inlined in both installer specs in addition to - the shared `require_ui_assets()` helper, because those specs are - deliberately self-contained (their own comments forbid importing the shared - datas module). -- Out of scope (observed, not changed): `except (ImportError, Exception)` at - utils/server.py get_network_addresses is redundant (Exception covers - ImportError) — pre-existing, harmless; `➜`/`⚠` may render double-width in - some Windows fonts (cosmetic; ASCII opt-ins now cover it). - ---- - -# Codex TUI adoption — Phase 1 (foundation) - -Source of truth: `blackbox/codex-main/codex-rs/tui`. Backlog: user-provided 48-item -adoption list. Recon mapped every item to the real codebase first; many items -already exist. This plan implements the genuine HIGH-priority gaps only. - -## Gap analysis (backlog item → reality) - -Already implemented (no work needed; documented for the record): -- 4.2 Table holdback — `markdown_commit_boundary()` keeps the last top-level - block (incl. tables) mutable during streaming (components/markdown.py:610). -- 4.3 Two-region streaming — Rich `Live(transient=True)` + scrollback commit - (`_ContentBlock._flush_committed`, visualize/_blocks.py:393). -- 4.1 Adaptive chunking — backlog-proportional paced reveal already adapts step - size to backlog (`reveal_tick`, _blocks.py:270); continuous policy, no mode - oscillation to dampen. Skipping the Rust two-gear port (no user-visible win). -- 7.4 Language aliases — Pygments already resolves py/js/ts/rs/sh/zsh/yml/golang. -- 9.1 OSC 8 hyperlinks — `render_to_ansi` wraps OSC 8 for prompt_toolkit - (console.py:94-123) with tests. -- 6.3 Exploring cells — ctrl+o expand/collapse on tool cards. -- 5.2/5.4 Shimmer + reduced motion — motion.py honors PYTHINKER_REDUCED_MOTION. -- 8.3 Grapheme/cell-aware truncation — render_utils.py uses rich cell_len. -- 3.x markdown styling, 2.x diff context collapsing, 6.1 cards — present. - -## Phase 1 work items - -- [x] P1.0 Recaps off by default + `/config recaps on|off` - → verified: tests/ui_and_conv/test_settings_recaps_slash.py (5 tests). -- [x] P1.1 Color-blend utilities (`ui/color_utils.py`): parse_hex/blend/luma/is_light. - → verified: tests/ui_and_conv/test_color_utils.py. -- [x] P1.2 Three-tier color depth detection (truecolor/256/16/none; FORCE_COLOR - levels; WT_SESSION promotion); `get_diff_colors()` uses fg-only diff - styles on 16-color terminals. - → verified: env-matrix tests in test_terminal_capabilities.py. -- [x] P1.3 Terminal background probing (`ui/terminal_background.py`): OSC 11, - 100ms timeout, BT.601 luma; `theme = "auto"` resolved at shell startup - (fallback dark); /theme + settings selector accept "auto"; opt-out - PYTHINKER_NO_BG_PROBE. - → verified: tests/ui_and_conv/test_terminal_background.py (11 tests). -- [x] P1.4 Syntax-highlight size guard (512 KiB / 10k lines → plain text + - "highlighting skipped (N lines)" title notice). - → verified: test_markdown_guards.py. -- [x] P1.5 Fence unwrapping for tables (```md/```markdown + header+delimiter - pair → unwrap; everything else untouched). - → verified: 12-case matrix in test_markdown_guards.py. -- [x] P1.6 Large diff guard: expanded diff capped at 400 lines, head+tail with - explicit omitted-count notice. - → verified: test_output_guards.py. -- [x] P1.7 Head-tail truncation for generic tool output (was head-only). - → verified: test_output_guards.py. -- [x] Focused tests: 143 passed; make check-pythinker-code all green. -- [ ] Full `pytest tests` suite green. -- [ ] Homebrew updater bug: `brew upgrade` runs against a stale tap and the - "already installed" warning is reported as "Updated successfully!". - Fix: refresh tap/brew before upgrade + verify installed version changed. - -## Out of scope (logged) - -- Phase 2/3 backlog items (per-hunk diff syntax highlighting, advanced table - column sizing/key-value fallback, custom theme files, animation variants, - compact JSON, URL-aware wrap, transcript export, HistoryCell protocol). -- utils/string.py `shorten()` is not cell-aware (pre-existing; noted, untouched). -- `/settings show` usage string update beyond the new recaps args. - ---- - -# Alibaba Token Plan model compatibility fix - -## Plan - -- [x] Reproduce the Kimi-only request-shaping regression with a focused test. -- [x] Preserve Moonshot/Z AI request formats for non-Alibaba providers. -- [x] Confirm the generic Token Plan `/models` response does not validate `sk-ws-` credentials. -- [x] Require the dedicated workspace Base URL for `sk-ws-` login. -- [x] Re-test Kimi K2.6 and DeepSeek V3.2 request behavior for the dedicated endpoint. -- [x] Fix the shell wire coroutine warning. -- [x] Run focused Alibaba auth/LLM tests and `make check-pythinker-code`. - -## Acceptance criteria - -- Alibaba Token Plan Kimi K2.6 sends `extra_body.enable_thinking`, not - `extra_body.thinking.type` or `reasoning_effort`. -- `sk-ws-` login requires and saves the dedicated Base URL shown in the Token Plan console. -- Login cannot falsely succeed based only on the generic endpoint's public `/models` response. -- DeepSeek V3.2 either produces a valid response or is excluded with evidence that the workspace - endpoint does not support it. -- Existing generic, regional, Coding Plan, Moonshot, and GLM behavior remains covered. - -## Review - -- Token Plan keys now require an explicit dedicated endpoint and the shell passes that endpoint - directly to the login flow. -- Regional fallback only saves a provider after the fallback endpoint authenticates successfully; - custom endpoints are never silently replaced. -- Workspace Kimi entries are filtered when the advertised route is unusable, while DeepSeek V3.2 - uses the verified non-streaming request path and DashScope `enable_thinking`. -- Final focused agent-spec, auth, LLM, and shell tests: `122 passed`. -- `make check-pythinker-code` passed. -- Full Pythinker Code test target passed: `4427 passed, 6 skipped, 1 xfailed`; wire E2E - passed: `52 passed, 4 skipped`. - ---- - # Plan: Full rename `pythinker-cli` → `pythinker-code` **Status**: Planning — DO NOT execute yet. User to review and approve. @@ -299,544 +138,103 @@ This is the biggest mechanical change. Use `sed` for the bulk pass, then verify grep -rn "pythinker_cli" --include="*.py" src/ packages/ sdks/ tests/ tests_e2e/ tests_ai/ scripts/ examples/ \ | grep -v "pythinker_cli_session" ``` - Expect: empty output. Any hits are either: - - Comments referencing the old name (ok to leave or update inline) - - Strings that intentionally hold the old name (e.g. backward-compat fallbacks) + Expect: empty output. -- [ ] **Handle `pythinker_cli_session` attribute renames separately** — these are part of the property API, not imports: +- [ ] **Handle `pythinker_cli_session` attribute renames separately**: ```bash find src tests -name "*.py" -type f \ -exec sed -i 's/\bpythinker_cli_session\b/pythinker_code_session/g' {} + ``` - Verify that the dataclass/typed-dict that *defines* this attribute also got renamed (likely in `src/pythinker_code/web/runner/worker.py` or similar). - [ ] **Run the test collector** to confirm all imports resolve: ```bash uv sync --frozen --all-extras --all-packages uv run pytest tests --co -q 2>&1 | tail -20 ``` - Expect: tests collect without `ModuleNotFoundError`. - -- [ ] **Spot-check import correctness** for known critical files: - - `src/pythinker_code/__main__.py` - - `src/pythinker_code/cli/__init__.py` - - `src/pythinker_code/cli/__main__.py` - - `src/pythinker_code/web/api/sessions.py` - - `src/pythinker_code/telemetry/sentry.py` (regex must be updated to match new path) - [ ] **Update telemetry path regex** in `sentry.py`: ```python r"^(.*?)(site-packages|pythinker_code|src/pythinker_code)/" ``` - (Keep `pythinker_cli` if you want backward-compat for older stack frames, but it shouldn't be needed now that we control the codebase.) -- [ ] Commit: - ```bash - git commit -am "refactor: rewrite pythinker_cli imports to pythinker_code" - ``` +- [ ] Commit. --- ### Phase 3 — pyproject.toml + workspace surgery (45 min) -Three pyprojects change roles. The current state: - -| File | Today | After | -|------|-------|-------| -| `pyproject.toml` (root) | declares `pythinker-cli` w/ all deps + scripts | declares `pythinker-cli` (thin alias depending on `pythinker-code==1.1.0`); minimal stub | -| `packages/pythinker-code/pyproject.toml` | declares `pythinker-code` w/ alias dep on `pythinker-cli==1.0.0` | becomes the canonical package: full deps, scripts, web/vis bundled, classifiers, urls | -| `packages/pythinker-cli/pyproject.toml` | does not exist | NEW — moved from root, but role-flipped to be the alias | - -Wait — this is confusing. Let me restate the cleaner approach. - -**Cleaner approach:** Don't ship `pythinker-cli` at all in 1.1.0. Just retire it. - -- [ ] **Move root pyproject contents** to `packages/pythinker-code/pyproject.toml`: - - Copy `dependencies`, `dependency-groups`, `[project.scripts]`, `[project.urls]`, `classifiers`, `keywords` from root into `packages/pythinker-code/pyproject.toml` - - Update `name = "pythinker-code"`, `version = "1.1.0"` (semver bump for breaking change in package layout) - - Keep `module-name = ["pythinker_code"]` in `[tool.uv.build-backend]` - - Add `license`, `license-files`, `authors` (already there from earlier work) - -- [ ] **Decide root pyproject's fate** — pick one: - - **Option A**: Delete root `pyproject.toml` entirely, move workspace config into a new top-level file. (Cleanest but might break tooling.) - - **Option B**: Keep root `pyproject.toml` as a **dev/workspace-only** file with no package; uv can still treat the repo root as a workspace coordinator. Set `[project] name = "pythinker-monorepo"` private (do not publish). - - **Option C** (recommended): Keep root `pyproject.toml` as a thin alias for `pythinker-cli==1.1.0` that just `dependencies = ["pythinker-code==1.1.0"]`. Ships ONCE in 1.1.0 to give existing pythinker-cli users a deprecation upgrade path. Skip in 1.2.0+. - - Recommend **Option C** for migration clarity. - -- [ ] **Update `[tool.uv.workspace]`** members list — depends on Option chosen. -- [ ] **Update `[tool.uv.sources]`** to reflect new dependency wiring. -- [ ] Update `module-name` in root pyproject's `[tool.uv.build-backend]` if Option C — point it at a stub directory (or remove the section if there's no module to build). - -- [ ] **Adjust `[project.scripts]`**: - - In `packages/pythinker-code/pyproject.toml`: - ```toml - [project.scripts] - pythinker = "pythinker_code.__main__:main" - pythinker-cli = "pythinker_code.__main__:main" # legacy alias for one release - pythinker-code = "pythinker_code.__main__:main" - ``` - -- [ ] **Build all packages locally**: - ```bash - rm -rf dist/ packages/*/dist/ sdks/*/dist/ - uv build --package pythinker-code --no-sources --out-dir dist - uv build --package pythinker-core --no-sources --out-dir dist - uv build --package pythinker-host --no-sources --out-dir dist - uv build --package pythinker-sdk --no-sources --out-dir dist - # If Option C: - uv build --package pythinker-cli --no-sources --out-dir dist - ``` - All five must succeed. - -- [ ] Commit: - ```bash - git commit -am "refactor: pyproject swap - pythinker-code as canonical, pythinker-cli as alias" - ``` +- [ ] **Move root pyproject contents** to `packages/pythinker-code/pyproject.toml` +- [ ] **Decide root pyproject's fate** — Option C (recommended): keep as thin `pythinker-cli==1.1.0` alias for one release +- [ ] **Update `[tool.uv.workspace]`** members list +- [ ] **Adjust `[project.scripts]`** in `packages/pythinker-code/pyproject.toml` +- [ ] **Build all packages locally** +- [ ] Commit. --- ### Phase 4 — Build/release infrastructure (30 min) -- [ ] **Update `pythinker.spec` (PyInstaller)**: - ```python - from pythinker_code.utils.pyinstaller import datas, hiddenimports - # ... - ["src/pythinker_code/cli/__main__.py"], - ``` -- [ ] **Update `Makefile`** target body (Makefile target *names* can stay: `build-pythinker-cli` is just an internal alias, but rename for consistency): - ```makefile - build-pythinker-code: build-web build-vis - @uv build --package pythinker-code --no-sources --out-dir dist - ``` - Keep `build-pythinker-cli` as a deprecated alias that calls the new target if you want to avoid breaking developer muscle memory. -- [ ] **Update `scripts/build_web.py`** if it copies output into `src/pythinker_cli/web/...` — change to `src/pythinker_code/web/...`. -- [ ] **Update `scripts/build_vis.py`** likewise. -- [ ] **Update `scripts/check_pythinker_dependency_versions.py`** to validate `pythinker-code` vs old name. -- [ ] **Local sanity build**: - ```bash - make build-pythinker-code - ls dist/ # verify pythinker_code-*.whl present - ``` -- [ ] **Local PyInstaller dry run** (catches the most catastrophic class of breakage early): - ```bash - PYINSTALLER_ONEDIR=1 make build-bin-onedir - dist/onedir/pythinker/pythinker --version # should print 1.1.0 - ``` +- [ ] Update `pythinker.spec` (PyInstaller) +- [ ] Update `Makefile` targets +- [ ] Update `scripts/build_web.py` and `scripts/build_vis.py` +- [ ] Update `scripts/check_pythinker_dependency_versions.py` +- [ ] Local PyInstaller dry run - [ ] Commit. --- ### Phase 5 — Workflow files & PyPI publisher records (45 min) -**Decision**: Keep the workflow filenames as `release-pythinker-cli.yml` for one release cycle to avoid re-registering PyPI publishers, then rename to `release-pythinker-code.yml` in a follow-up. - -- [ ] **Edit `.github/workflows/release-pythinker-cli.yml`**: - - Update `make build-pythinker-cli` → `make build-pythinker-code` - - Update `environment.url` to `https://pypi.org/project/pythinker-code/` - - Validate-tag step: still uses root `pyproject.toml` version OR switch to `packages/pythinker-code/pyproject.toml` (depends on Option chosen in Phase 3) -- [ ] **Update `scripts/check_version_tag.py` callsites** in workflow if pyproject paths changed. -- [ ] **Update `scripts/check_pythinker_dependency_versions.py` callsite**. -- [ ] **PyPI dashboard work** (manual via Chrome MCP or browser): - - Visit https://pypi.org/manage/project/pythinker-code/settings/publishing/ - - Confirm the pending publisher (`release-pythinker-cli.yml`, env=pypi) is still there. After 1.1.0 publishes, it'll convert to active. - - If using Option C (keep cli alias for one release): confirm pythinker-cli's existing publisher still references `release-pythinker-cli.yml` with env=pypi. It will fire on 1.1.0 tag. +- [ ] Edit `.github/workflows/release-pythinker-cli.yml` (keep filename, update content) +- [ ] Update `scripts/check_version_tag.py` callsites +- [ ] Verify PyPI dashboard trusted publishers still valid - [ ] Commit. --- ### Phase 6 — Documentation, examples, agent YAMLs (60 min) -This is mostly mechanical sed-replace, but every file needs a quick eyeball pass to make sure the rename reads naturally in prose. - -- [ ] **README.md**: Replace `pythinker-cli` → `pythinker-code` in install commands, badges, package references. Lead the install section with `pip install pythinker-code`. -- [ ] **CONTRIBUTING.md**, **SECURITY.md**, **CHANGELOG.md**: Update package references. -- [ ] **`docs/en/**/*.md`**: 15+ files. Bulk sed first, then read each for prose oddity: - ```bash - find docs -name "*.md" -exec sed -i 's/pythinker-cli/pythinker-code/g; s/pythinker_cli/pythinker_code/g' {} + - ``` -- [ ] **`AGENTS.md`** (top-level + nested): same treatment. -- [ ] **`.agents/skills/**/*.md`**: same. -- [ ] **`tasks_ai/**/*.md`**: same. -- [ ] **`examples/**`** (60 references): - - Update each `pyproject.toml` dependency line: `"pythinker-cli==1.0.0"` → `"pythinker-code==1.1.0"` - - Update example READMEs and yaml files -- [ ] **Agent YAMLs** in `src/pythinker_code/agents/default/*.yaml`, `okabe/agent.yaml`: - - Update tool import paths: `"pythinker_cli.tools.shell:Shell"` → `"pythinker_code.tools.shell:Shell"` (×30+) - - These are CRITICAL — wrong paths cause runtime tool-loading failures, often only when a specific tool is invoked -- [ ] **`web/openapi.json` and `web/package.json`**: Update package name references. -- [ ] **`web/src/lib/api/docs/ConfigApi.md`**: doc reference. -- [ ] **Skill markdown** (`src/pythinker_code/skills/pythinker-cli-help/SKILL.md`): rename directory itself to `pythinker-code-help/` and update internal references. -- [ ] **Pre-commit config** `.pre-commit-config.yaml`: any path filters? -- [ ] **`.python-version`, `flake.nix`, `flake.lock`**: scan for references. +- [ ] README.md, CONTRIBUTING.md, CHANGELOG.md +- [ ] `docs/en/**/*.md`, AGENTS.md, skills, tasks_ai +- [ ] `examples/**` (60 references — pyproject + READMEs + yamls) +- [ ] Agent YAMLs: tool import paths `"pythinker_cli.tools.*"` → `"pythinker_code.tools.*"` - [ ] Commit. --- ### Phase 7 — Verification (60 min) -- [ ] **Full test suite**: - ```bash - uv run pytest tests -v 2>&1 | tail -50 - ``` - Expect: same pass/fail rate as `pre-rename-snapshot`. Compare: - ```bash - git diff pre-rename-snapshot HEAD --stat -- 'tests/**' 'src/**' - ``` -- [ ] **`uv sync` clean**: - ```bash - rm -rf .venv uv.lock - uv sync --frozen=false --all-extras --all-packages - ``` -- [ ] **Type check**: - ```bash - uv run pyright src/ - uv run ty check - ``` - Expect: no new errors. -- [ ] **Ruff/lint**: - ```bash - uv run ruff check - uv run ruff format --check - ``` -- [ ] **Smoke import**: - ```bash - uv run python -c "import pythinker_code; print(pythinker_code.__file__)" - uv run python -c "from pythinker_code.cli import main" - ``` -- [ ] **Run the CLI**: - ```bash - uv run pythinker --version # prints 1.1.0 - uv run pythinker --help # full help text - uv run pythinker-code --help # alias works - ``` -- [ ] **PyInstaller binary** (the most common late-stage failure): - ```bash - rm -rf build/ dist/onedir/ dist/onefile/ - PYINSTALLER_ONEDIR=1 make build-bin-onedir - dist/onedir/pythinker/pythinker --version # 1.1.0 - ``` -- [ ] **Web UI build** (if applicable): - ```bash - npm --prefix web run build - ``` -- [ ] **TestPyPI dry run** before PyPI: - ```bash - uv build --package pythinker-code --no-sources --out-dir dist - uvx twine upload --repository testpypi dist/pythinker_code-1.1.0* - ``` - Then smoke install: - ```bash - python -m venv /tmp/v && /tmp/v/bin/pip install \ - --index-url https://test.pypi.org/simple/ \ - --extra-index-url https://pypi.org/simple/ \ - pythinker-code==1.1.0 - /tmp/v/bin/pythinker --version - ``` +- [ ] Full test suite vs pre-rename-snapshot +- [ ] `uv sync` clean (rm .venv + uv.lock) +- [ ] pyright + ruff +- [ ] Smoke import + CLI run +- [ ] PyInstaller binary +- [ ] TestPyPI dry run --- ### Phase 8 — Tag and release (30 min) -- [ ] Update `CHANGELOG.md` with 1.1.0 entry: rename, breaking changes, migration notes for users coming from `pythinker-cli==1.0.0`. -- [ ] Squash-merge `rename/pythinker-code` to `main` (or fast-forward if commits are clean): - ```bash - git switch main - git merge --ff-only rename/pythinker-code - git push origin main - ``` -- [ ] Tag and push: - ```bash - git tag -a 1.1.0 -m "v1.1.0: rename pythinker-cli to pythinker-code" - git push origin 1.1.0 - ``` -- [ ] **Watch the workflow**: - ```bash - gh run watch $(gh run list --workflow=release-pythinker-cli.yml --limit 1 --json databaseId --jq '.[0].databaseId') --exit-status - ``` -- [ ] **Verify on PyPI**: - ```bash - pip index versions pythinker-code # 1.1.0 - pip index versions pythinker-cli # 1.0.0 + 1.1.0 (alias release) - ``` -- [ ] **Smoke install in clean venv**: - ```bash - python -m venv /tmp/v-final && /tmp/v-final/bin/pip install pythinker-code==1.1.0 - /tmp/v-final/bin/pythinker --version - ``` +- [ ] CHANGELOG 1.1.0 entry with migration notes +- [ ] Squash-merge to main and push tag +- [ ] Watch release workflow +- [ ] Verify on PyPI --- ### Phase 9 — Post-release cleanup (deferred to 1.2.0) -To do later, in a separate PR after we know 1.1.0 is healthy: - -- [ ] Drop the `pythinker-cli` alias package — stop publishing it -- [ ] Rename workflow files: `release-pythinker-cli.yml` → `release-pythinker-code.yml` and update PyPI publisher records -- [ ] Remove `pythinker-cli` script entry from `[project.scripts]` -- [ ] Update root README to remove the migration callout -- [ ] Bump everything to 1.2.0 - ---- - -## Out of scope - -- Renaming `pythinker-core`, `pythinker-host`, `pythinker-sdk` packages (their names are already fine) -- Renaming the GitHub repo itself (`Pythoughts-labs/pythinker-code`) — name is already correct -- Breaking the public Python API (we're keeping `import pythinker_code` ergonomic; the *internal* import path changes but the module's public API surface stays the same) -- Migrating user data directories (the rename creates a new path, but no existing user data exists yet — by user statement) - ---- - -## Risks and rollback - -If anything goes catastrophically wrong: - -```bash -# On the rename branch, before merging to main: -git switch main # back to clean state, rename branch unaffected - -# After merging, if 1.1.0 is broken on PyPI: -# Yank the broken release (don't unpublish — that's permanent) -twine ... # PyPI doesn't have CLI yank; do it via dashboard -``` - -PyPI 1.0.0 of pythinker-cli stays published forever. Anyone who installed it before 1.1.0 ships keeps working. 1.1.0 of pythinker-cli (the alias) and pythinker-code (canonical) ship together. - ---- - -## Success criteria - -- [ ] `pip install pythinker-code` from real PyPI in a clean venv succeeds -- [ ] `pythinker --version` prints `1.1.0` -- [ ] `import pythinker_code` works; `import pythinker_cli` does NOT (after 1.2.0) -- [ ] PyInstaller binary on GitHub Releases for v1.1.0 runs and prints `1.1.0` -- [ ] All tests pass at the same rate as `pre-rename-snapshot` -- [ ] No new pyright/ty errors -- [ ] Documentation (README, docs/, examples/) leads with `pythinker-code` everywhere +- [ ] Drop `pythinker-cli` alias package +- [ ] Rename workflow file + re-register PyPI publishers +- [ ] Remove `pythinker-cli` script entry +- [ ] Bump to 1.2.0 --- ## Open questions — ANSWER BEFORE EXECUTION -1. **Do you want to ship `pythinker-cli==1.1.0` as a one-shot deprecation alias** (Option C in Phase 3), or **drop it cold-turkey at 1.1.0** (Option A/B)? Cold-turkey is simpler but anyone who happened to grab `pythinker-cli==1.0.0` won't be migrated automatically. - -2. **Workflow filename**: keep `release-pythinker-cli.yml` for one release (no PyPI publisher changes needed), or rename to `release-pythinker-code.yml` immediately (requires re-registering PyPI publishers, which we just did once and rate-limited)? - -3. **Module name**: confirm `pythinker_code` is what you want for the Python module. Alternative: `pythinker` alone (cleaner but might collide with random PyPI projects). - -4. **Migration text in README**: Do you want a "migrating from pythinker-cli" callout in 1.1.0's README, or just silently switch? - -5. **CHANGELOG framing**: Is this a breaking change that warrants 2.0.0, or a layout change that's fine at 1.1.0? PyPI users perspective: install command changed, that's user-visible breakage. Could argue 2.0.0. - ---- - -# Web fetch/search domain allowlist (2026-05-27) - -Port of the one genuinely portable concept from pythinker-x's web search -(`allowed_domains`) onto our self-hosted FetchURL/SearchWeb tools. Design spec: -`docs/superpowers/specs/2026-05-27-web-allowed-domains-design.md`. - -- [x] `WebConfig.allowed_domains` config (+ field validator rejecting URLs/paths/host:port) -- [x] `host_in_allowlist` helper (label-aware subdomain match, unrestricted when empty) -- [x] FetchURL: reject out-of-allowlist hosts in `_validate_fetch_url` (no request made) -- [x] SearchWeb: post-filter results, surface dropped count via `extras` -- [x] TUI: muted "· N filtered to allowlist" indicator on the search result header -- [x] Tests: helper, config validation, fetch rejection, search filter, renderer indicator -- [x] Docs: `docs/en/configuration/config-files.md` `web` section + example - -## Review -- All affected suites green (tools/core/ui = 2517 passed earlier; affected subset 100 passed). -- ruff + ruff format clean; pyright clean on all changed files. The 8 pre-existing - pyright errors live in `cli/mcp.py` and `soul/toolset.py` (untouched, baseline). -- Dropped from scope (cosmetic/redundant in our architecture): action taxonomy relabel, - disabled/cached/live mode gating. See design doc "Out of scope". - -## Out of scope (observed, not changed) -- Pre-existing pyright errors in `cli/mcp.py`, `soul/toolset.py`. - -## Review follow-up (2026-05-27) — context7-validated hardening -Reviewed the allowlist against context7 (aiohttp v3.13.2, pydantic v2) + 2026 agent-tool practice. -- [x] HIGH: redirect bypass — `fetch_with_http_get` now sets `allow_redirects=False` and follows - redirects manually (max 5), re-validating each hop via `_validate_fetch_url` (allowlist + - SSRF). Closes a pre-existing SSRF gap the allowlist had inherited. Tests: follows validated - redirect, blocks redirect to disallowed host (never contacted), rejects redirect loop. -- [x] LOW: `fetch.md` / `search.md` now state the allowlist constraint to the model. -- [x] LOW: `WebConfig` validator now rejects empty/whitespace-only entries (was silently unrestricted). -- Confirmed-good (context7): pydantic validator matches docs exactly; `extras` TUI channel; - fail-closed on unparsable hosts; allowlist-before-DNS ordering. -- Known/accepted limitation (pre-existing, not addressed): DNS-rebinding TOCTOU — `_validate_fetch_url` - resolves+checks IPs but aiohttp re-resolves at connect time. Out of scope; would need a pinning connector. -- Snapshots updated: `test_default_config_dump`, `test_fetch_url_description`, `test_search_web_description`. - ---- - -## Review: code-review `/diff` findings — robust fixes (this session) - -`/code-review` (xhigh) on `feat/agent-phase0-enhancements` surfaced 12 findings; -research-backed (OWASP LLM Top 10, Python asyncio docs, ACP spec) TDD fixes applied. -Each fix: failing test first → minimal change → green. Full gate: 4682 unit + 65 e2e -pass; ruff + project-wide pyright clean. - -### Security (HIGH) -- **#1 `soul/approval.py` approve-for-session drain → destructive sibling.** Stored an - authoritative `session_approvable` flag on `ApprovalRequestRecord` at create time (from - the real tool_call, not reconstructed from display blocks); both drains skip - non-session-approvable pending siblings. `rm ` approval can no longer clear a - queued `rm -rf`. (models.py + runtime.py + approval.py) -- **#2 `utils/path.py` `.md` agent specs escaped EDIT_CONFIG.** Added `.md` to the - agent-spec-dir config-surface check; markdown subagent specs now re-confirm like YAMLs. - -### Correctness (MODERATE) -- **#5 `soul/approval.py` unattended fail-closed hole.** `_unattended_denial_feedback` now - re-derives the two downstream auto-resolve conditions and denies anything that would - otherwise block forever — closes the safe-mode destructive-shared-key hang AND the - config-edit-in-non-safe-auto hang (same class, fixed beyond the original finding). -- **#4 `acp/convert.py` `` leaked to ACP/IDE.** Strip the envelope at the - ACP output boundary (ACP defines no untrusted-output marking — we sanitize ourselves). -- **#3 `background/agent_runner.py` child usage roll-up.** Added `output.usage(...)` so a - background child's `child_tokens:`/`child_cost_usd:` ride in its transcript. - **Limitation:** this surfaces spend in the *TaskOutput transcript* only; - `summarize_batch` aggregates launch-time stub results, so the structured parent roll-up - (`total_child_tokens`) still excludes background children. Deeper fix = pull child - `extras` from the completed background result; deferred. - -### Low / efficiency / cleanup -- **#6 `soul/toolset.py`** narrow MCP capability-discovery errors: METHOD_NOT_FOUND = - expected/empty/debug; anything else = WARNING (transient ≠ "no capability"); deduped. -- **#7 `tools/utils.py`** `async spill_to_disk()` offloads the on-truncation write via - `asyncio.to_thread` (idempotent; sync fallback preserved) + atomic temp+os.replace - (cancellation can't leave a partial recovery file). Wired into Shell/FetchURL/SearchWeb. -- **#8/#9 `memory/recall.py`** arm `_injected`/baselines only after a successful snapshot - (transient failure retries instead of latching a stale baseline); defer the working-set - scan behind the cheap turn-throttle gate. -- **#10 `soul/pythinkersoul.py`** prune anchors the token count to `before_tokens` minus - the estimated freed delta (same estimator both sides → bias cancels) instead of a full - re-estimate that could over-count and re-fire the rewrite every step. -- **#11 `soul/pythinkersoul.py`** extracted `_opt_int` for the 4 repeated usage ternaries. - -### Declined (with rationale) -- **#12 `model_defense.py` `excludes` field.** KEPT — it is tested - (`test_fragment_matches_with_patterns_and_excludes`) and a deliberate, documented - extension point in a registry built to grow; removing tested behavior isn't a clean - simplification (surgical-changes > YAGNI here, negligible cost). - -### Follow-up: investigated + fixed the concurrent OpenAI-feature changes (user-directed) -A concurrent (paused) WIP appeared in the tree during the review session — ChatGPT 429 -usage-limit messaging + `/login` account-switch detection (auth/openai.py, chat_provider, -ui/shell). Investigated properly: feature logic is correct and its tests pass. Two real -issues fixed (TDD): -- **Markup-escape bug** `ui/shell/__init__.py`: 429 summary/hint were interpolated into a - Rich-markup string unescaped, so a provider message containing `[...]` was silently - dropped. Extracted `_render_429_message(detail)` that `escape()`s both fields (matches - the sibling error branches); handler now calls it. New test in test_rate_limit_message.py. -- **Flaky test** `tests/auth/test_openai_auth.py`: the two `_wait_for_browser_code` callback - tests used tight 2s/0.05s timing deadlines that flake under CPU load (clean TimeoutError; - load-correlated; the suspected port-leak order passes 10/10). Prod OAuth ports are fixed - and can't change, so the fix is test-only: a generous `_BROWSER_CALLBACK_TEST_TIMEOUT` - for the connect/await deadlines and a bounded poll-until-done instead of a fixed sleep. - Originally-flaky combo now 6/6 stable under random ordering; tests/auth+ui_and_conv 1822 pass. - -## Review — session 2026-06-09 (Phase 1 + design polish + hardening) - -Done beyond Phase 1 (user-directed design wave): -- ⏺ transcript marker (Windows keeps ●, ASCII keeps *), blinking while - tools/preview run, solid green when finished; thinking rows use ⏺ too. -- Muted clay-coral activity ramp; shimmer simplified to bidirectional sweep - with settle beats; truecolor gets cosine-blended sheen (color_utils.blend). -- Activity/todo metadata unified: "Verb… (12s, ↓ 2.4k tokens, 45 t/s)" — no - middle dots; t/s counter added to working indicator + todo header. -- Thinking-effort colors: cold→hot gradient (slate→blue→teal→amber→orange→ - dark red xhigh). -- Active todo title+box coral; concurrent in-progress rows light grey. -- Diff word-level highlights: reverse-video → theme add/del highlight bgs. -- Turn recap padded to card inset. -- Hardening (validated from in-app review): select ValueError caught, - tcsetattr restore suppressed, probe reply byte cap, probe cache lock, - head/tail char budget halved per side, mode.split() once, ~~~md fence test. -- Homebrew updater: user machine upgraded 0.38.0→0.39.0 (stale tap); the - code fix already shipped in v0.39.0 (PR #87). - -Verified: make check-pythinker-code green; full pytest tests: 4757 passed. - -## Out of scope / next (designs ready, not implemented) - -- /statusline command (Codex bottom_pane/status_line_setup.rs): config key - tui.status_line list[str], item registry (model, current-dir, git-branch, - context-remaining, used-tokens), wire into prompt.py - _render_card_bottom_toolbar; recon notes in session memory. -- Background working status: replace "N background agents" suffix with - (elapsed, tokens, t/s) — needs an elapsed/tokens provider on - CustomPromptSession (footer already shows the bg count). -- Slash-command audit verdict: nothing safely removable — /exit is - Shell-intercepted (completion needs the registry entry); /color,/status, - /cost,/config are deliberate Blackbox-style aliases guarded by - test_blackbox_style_slash_aliases_are_registered. Optional renames - (/sessions→/resume primary, /memory→/memories) left to user choice. -- Phase 2/3 backlog: per-hunk diff syntax highlighting, Codex table column - sizing + key/value narrow fallback, per-file multi-file diff summaries, - URL-aware wrap, custom themes, compact JSON, transcript export. - -## Review — 2026-06-10 deep-scan remediation + robustness pass - -Deep multi-agent review of feat/tui-enhancements (9 finder angles → 1-vote -verify → sweep), then fixes landed for every confirmed finding, plus the -multi-instance/orchestration robustness directive and the openai.py package -split. Highlights: - -- Security: awk pipe/getline + xargs -L permission bypasses closed (both - gates); Glob symlink-escape fixed; progress-note title ANSI-sanitized. -- Correctness: grep field-separator parsing (digit-hyphen paths), CRLF - multi-line replace fallback, /import raw-path --force parsing, compaction - reminders for --add-dir files, double-cancel shield settle, replay - watermark stat-fail → full replay, agent-resume ValueError, oauth - refresh_token/expires_in/device-id hardening, /theme auto re-probe, - markdown fence close-with-info-string, MCP cross-server shadow warning. -- Multi-instance: per-session writer flock (.owner.lock), locked - pythinker.json mutate helper, JSONL torn-line repair, atomic fork writes, - strict memory reads (no wipe-on-EIO), journal cap (100), atomic inbox - claim, recall mtime re-arm, scratchpad/memory flock via to_thread. -- Orchestration: continuation failure keeps completed results; hallucinated - subagent types fail fast w/ valid list (Agent + RunAgents); background - failures carry Agent ID + resume hint; finalize guarded; runner crashes - logged via done-callback; copy_for_role shares live-task registry. - -### Out of scope / deferred (next branch candidates) -- Retry-After-aware backoff + larger background retry budget (O3) and - foreground timeout inside the runner with usage+resume hint (O4). -- Scratch cap rewrite via mkstemp+replace (M4); _ensure_dir share-dir - re-resolution (M7); snapshot per-section budget clamp (M8). -- Token-rate tracker triplication (prompt.py/_live_view/_blocks), head/tail - truncation + fence-walker dedup, todo-glyph mapping hoist, bullet factory. -- web config API: optional private-range carve-out for plain-HTTP LAN - providers (currently CLI-only flow, deliberate). - -## 2026-06-10 — ESC interrupt + recall hallucination fixes (fix/web-origins-banner-version) - -Root causes (investigated from real session 243fa26d + code trace): -- "ping" hallucination: RecallInjectionProvider injected "Open todos from - recent sessions" + scratch notes containing imperatives ("Wait for both to - complete, then synthesize") with no "this is history, not an instruction" - framing → model treated it as the current task. -- ESC: shell RunCancelled handler only prints "Interrupted by user". - Background tasks spawned during the turn keep running - (kill_all_active exists but is never called on interrupt). - -Plan: -- [x] memory/recall.py: harden recall-block framing (header + open-todos - section) → verified: test_build_recall_block_frames_content_as_past_context. -- [x] background/manager.py: begin_turn / kill_turn_tasks turn registry - → verified: 2 new tests in tests/background/test_manager.py. -- [x] ui/shell/__init__.py: begin_turn before each run_soul; on RunCancelled - kill turn tasks + print count → verified: tests/ui_and_conv/test_shell_interrupt_cleanup.py. -- [x] web/src/bootstrap.tsx: consume ?token= BEFORE React mounts (was a React - effect racing useSessions' mount fetches → first-load 401 with stale - localStorage token). App.tsx effect removed. dist rebuilt. -- [x] Verification: 162 pytest green (background, recall, shell suites), - ruff + pyright clean on touched files, web tsc -b + biome clean. - -Review: ESC now kills background tasks spawned by the interrupted turn only -(earlier turns' tasks deliberately survive). Recall block is explicitly framed -as past context so stale todos can't be mistaken for the current request. -Out of scope (observed, not touched): vis frontend keeps token in URL (no -race); foreground subagent cancellation already correct via CancelledError. +1. **Alias or cold-turkey?** Ship `pythinker-cli==1.1.0` as a one-shot deprecation alias (Option C), or drop at 1.1.0? +2. **Workflow filename**: keep `release-pythinker-cli.yml` for one release, or rename immediately? +3. **Module name**: confirm `pythinker_code` (vs. bare `pythinker`)? +4. **Migration callout** in README? +5. **Versioning**: breaking layout change at 1.1.0 or 2.0.0? From c7be8a2623c44f09cd0c134007a7d21d035899bd Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Thu, 11 Jun 2026 00:23:26 +0000 Subject: [PATCH 2/9] fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit --- tasks/todo.md | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/tasks/todo.md b/tasks/todo.md index e951477b..dd516995 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -134,29 +134,23 @@ This is the biggest mechanical change. Use `sed` for the bulk pass, then verify ``` - [ ] **Verify no orphan `pythinker_cli` strings remain in *.py**: - ```bash - grep -rn "pythinker_cli" --include="*.py" src/ packages/ sdks/ tests/ tests_e2e/ tests_ai/ scripts/ examples/ \ - | grep -v "pythinker_cli_session" - ``` - Expect: empty output. -- [ ] **Handle `pythinker_cli_session` attribute renames separately**: ```bash - find src tests -name "*.py" -type f \ - -exec sed -i 's/\bpythinker_cli_session\b/pythinker_code_session/g' {} + + grep -rn "pythinker_cli" --include="*.py" src/ packages/ sdks/ tests/ tests_e2e/ tests_ai/ scripts/ examples/ ``` + Expect: empty output (all references should now be `pythinker_code`). + +- [ ] **Verify `pythinker_code_session` attribute usage**: + Confirm that attribute references like `joint_session.pythinker_code_session` and `session.pythinker_code_session` are correctly updated in web API and worker files. + - [ ] **Run the test collector** to confirm all imports resolve: + ```bash uv sync --frozen --all-extras --all-packages uv run pytest tests --co -q 2>&1 | tail -20 ``` -- [ ] **Update telemetry path regex** in `sentry.py`: - ```python - r"^(.*?)(site-packages|pythinker_code|src/pythinker_code)/" - ``` - - [ ] Commit. --- From c4c9625153411cc70947afc705029853c6b7cf44 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Wed, 10 Jun 2026 23:44:15 -0400 Subject: [PATCH 3/9] fix: harden shell, terminal, and plan-mode robustness - Shell error briefs now surface the trailing output of a failed command (last non-empty lines rendered as plain text), so the collapsed worklog card explains why a command failed instead of only its exit code. - Subagents no longer receive plan-mode workflow reminders; the reminder is root-only, since subagent toolsets usually exclude the plan-mode tools and the injection only invited calls to tools they don't have. - The terminal cursor-position probe no longer risks hanging in raw mode on exit: reads are non-blocking during the probe and VMIN/VTIME are restored to canonical defaults so a cancelled probe can't wedge the tty. - Reorder the welcome info block so session/storage details render last, and log the auto-save path failure instead of swallowing it silently. --- CHANGELOG.md | 4 + docs/en/reference/pythinker-vis.md | 2 +- docs/en/reference/slash-commands.md | 4 +- src/pythinker_code/acp/tools.py | 8 +- src/pythinker_code/app.py | 21 +- src/pythinker_code/auth/alibaba.py | 2 +- src/pythinker_code/auth/openai/models.py | 2 +- .../auth/openai/oauth_client.py | 2 +- src/pythinker_code/auth/platforms.py | 2 +- src/pythinker_code/cli/__init__.py | 2 +- src/pythinker_code/hooks/runner.py | 2 +- src/pythinker_code/llm.py | 10 +- src/pythinker_code/session_cleanup.py | 7 +- .../soul/dynamic_injections/plan_mode.py | 6 + src/pythinker_code/soul/pythinkersoul.py | 2 +- src/pythinker_code/tools/shell/__init__.py | 6 +- src/pythinker_code/tools/utils.py | 20 ++ src/pythinker_code/ui/color_utils.py | 3 +- src/pythinker_code/ui/shell/__init__.py | 251 ++++++++++++++---- .../ui/shell/components/markdown.py | 2 +- src/pythinker_code/ui/shell/mcp_status.py | 18 +- src/pythinker_code/ui/shell/motion.py | 2 +- src/pythinker_code/ui/shell/prompt.py | 10 +- .../ui/shell/render_constants.py | 2 +- src/pythinker_code/ui/shell/slash.py | 4 +- src/pythinker_code/ui/shell/stats_pricing.py | 2 +- .../ui/shell/usage_adapters/openai_chatgpt.py | 2 +- .../ui/shell/visualize/_worklog.py | 11 +- src/pythinker_code/ui/terminal_background.py | 5 +- .../ui/terminal_capabilities.py | 2 +- src/pythinker_code/ui/theme.py | 4 +- src/pythinker_code/utils/path.py | 2 +- src/pythinker_code/utils/term.py | 14 + .../core/test_plan_mode_injection_provider.py | 18 ++ tests/ui_and_conv/test_shell_switch_slash.py | 46 ++-- tests/ui_and_conv/test_shell_welcome_info.py | 97 ++++++- tests/ui_and_conv/test_worklog_render.py | 11 + tests/utils/test_result_builder.py | 44 +++ 38 files changed, 511 insertions(+), 141 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3dd29bc..d0608622 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **Shell error briefs now show the trailing output of a failed command.** When a `Shell`/`Terminal` command exits non-zero, times out, or is killed by a signal, the collapsed worklog card appended only `Failed with exit code: N`; you had to expand the result to see *why*. The brief now includes the last few non-empty output lines (e.g. the stderr message), rendered as plain text so shell metacharacters (backticks, `#`, `*`) and line breaks are preserved verbatim instead of being reflowed as Markdown. +- **Subagents no longer receive plan-mode workflow reminders.** Plan mode is a session-wide flag shared with subagents (so it persists across resume), but subagent toolsets usually exclude `EnterPlanMode`/`ExitPlanMode`. Injecting the plan-mode reminder into a subagent only invited hallucinated calls to tools it doesn't have; the reminder is now root-only. +- **Terminal no longer risks hanging in raw mode on exit.** The cursor-position probe left `stdin` in cbreak mode and could block in an uninterruptible `os.read()` if cancelled mid-probe (e.g. a race with prompt_toolkit's reader on shutdown). Reads are now non-blocking during the probe and `VMIN`/`VTIME` are restored to canonical defaults, so a hang or crash can't leave the terminal wedged. + ## 0.40.1 (2026-06-10) - **Windows/Linux native installers: web UI no longer 404s on `/`.** The installer CI froze the app without building the gitignored web/vis frontend bundles, so `pythinker web` opened a browser onto `GET /?token=… → 404 Not Found`. Both installer workflows now build the bundles before PyInstaller (matching the PyPI release flow — pip/wheel installs were never affected), every PyInstaller spec refuses to freeze when the bundles are missing, and a build that still lacks them serves an explanatory page on `/` (with the REST API still reachable under `/api`) instead of a bare 404. diff --git a/docs/en/reference/pythinker-vis.md b/docs/en/reference/pythinker-vis.md index 2ac77a3c..57ed8d29 100644 --- a/docs/en/reference/pythinker-vis.md +++ b/docs/en/reference/pythinker-vis.md @@ -18,7 +18,7 @@ The server automatically opens a browser after startup. The default address is ` If the default port is in use, the server will pick the next available port (by default `5495`–`5504`) and print the access URL in the terminal. -You can also type `/vis` in the interactive shell to switch directly from the current session to the Visualizer. +You can also type `/reports` in the interactive shell to switch directly from the current session to the Visualizer. ## Command-line options diff --git a/docs/en/reference/slash-commands.md b/docs/en/reference/slash-commands.md index 1382ed50..82aa150d 100644 --- a/docs/en/reference/slash-commands.md +++ b/docs/en/reference/slash-commands.md @@ -322,9 +322,9 @@ Auto mode skips all approval confirmations and removes the clarifying-question s Switch to Web UI. Pythinker Code will start a Web UI server and open the current session in your browser, allowing you to continue the conversation in the Web UI. See [Web UI](./pythinker-web.md) for details. -### `/vis` +### `/reports` -Switch to the Agent Tracing Visualizer. Pythinker Code will start the visualizer server and open the current session's tracing view in the browser, where you can inspect Wire event timelines, context messages, and usage statistics. See [Agent Tracing Visualizer](./pythinker-vis.md) for details. +Open session reports in the Agent Tracing Visualizer. Pythinker Code will start the visualizer server and open the current session's tracing view in the browser, where you can inspect Wire event timelines, context messages, and usage statistics. See [Agent Tracing Visualizer](./pythinker-vis.md) for details. ## Command completion diff --git a/src/pythinker_code/acp/tools.py b/src/pythinker_code/acp/tools.py index 58a2313d..37ac9716 100644 --- a/src/pythinker_code/acp/tools.py +++ b/src/pythinker_code/acp/tools.py @@ -149,20 +149,22 @@ async def __call__(self, params: ShellParams) -> ToolReturnValue: else "" ) + tail = builder.tail() + tail_suffix = f"\n{tail}" if tail else "" if timed_out: return builder.error( f"Command killed by timeout ({timeout_label}){truncated_note}", - brief=f"Killed by timeout ({timeout_label})", + brief=f"Killed by timeout ({timeout_label}){tail_suffix}", ) if exit_signal: return builder.error( f"Command terminated by signal: {exit_signal}.{truncated_note}", - brief=f"Signal: {exit_signal}", + brief=f"Signal: {exit_signal}{tail_suffix}", ) if exit_code not in (None, 0): return builder.error( f"Command failed with exit code: {exit_code}.{truncated_note}", - brief=f"Failed with exit code: {exit_code}", + brief=f"Failed with exit code: {exit_code}{tail_suffix}", ) return builder.ok(f"Command executed successfully.{truncated_note}") finally: diff --git a/src/pythinker_code/app.py b/src/pythinker_code/app.py index 42701709..75bde90f 100644 --- a/src/pythinker_code/app.py +++ b/src/pythinker_code/app.py @@ -789,7 +789,6 @@ async def run_shell( branch_name = _safe_git_branch(work_dir) if branch_name: welcome_info.append(WelcomeInfoItem(name="Branch", value=branch_name)) - welcome_info.append(WelcomeInfoItem(name="Session", value=self._runtime.session.id)) if notice := _resumed_unsupervised_notice( resumed=self._runtime.resumed, yolo=self._runtime.approval.is_yolo(), @@ -798,14 +797,6 @@ async def run_shell( welcome_info.append( WelcomeInfoItem(name="Mode", value=notice, level=WelcomeInfoItem.Level.WARN) ) - try: - auto_save_path = str( - shorten_home(HostPath.unsafe_from_local_path(self._runtime.session.context_file)) - ) - except Exception: - auto_save_path = "" - if auto_save_path: - welcome_info.append(WelcomeInfoItem(name="Auto-save", value=auto_save_path)) if base_url := self._env_overrides.get("PYTHINKER_BASE_URL"): welcome_info.append( WelcomeInfoItem( @@ -861,6 +852,18 @@ async def run_shell( level=WelcomeInfoItem.Level.WARN, ) ) + # Session persistence details come last — workspace and model identity + # read first, storage internals stay at the bottom of the facts block. + welcome_info.append(WelcomeInfoItem(name="Session", value=self._runtime.session.id)) + try: + auto_save_path = str( + shorten_home(HostPath.unsafe_from_local_path(self._runtime.session.context_file)) + ) + except Exception: + logger.debug("Failed to compute auto-save display path", exc_info=True) + auto_save_path = "" + if auto_save_path: + welcome_info.append(WelcomeInfoItem(name="Auto-save", value=auto_save_path)) welcome_info.append( WelcomeInfoItem( name="Tip", diff --git a/src/pythinker_code/auth/alibaba.py b/src/pythinker_code/auth/alibaba.py index 05fa5869..a825290d 100644 --- a/src/pythinker_code/auth/alibaba.py +++ b/src/pythinker_code/auth/alibaba.py @@ -218,7 +218,7 @@ def _infer_capabilities(model_id: str) -> frozenset[ModelCapability] | None: if _VISION_RE.search(mid) or _QWEN_PLUS_RE.search(mid) or "kimi" in mid: caps.add("image_in") if _REASONING_RE.search(mid): - # DeepSeek/Kimi expose a thinking dial; Qwen/GLM/MiniMax reason natively. + # DeepSeek/Moonshot expose a thinking dial; Qwen/GLM/MiniMax reason natively. caps.add("thinking" if ("deepseek" in mid or "kimi" in mid) else "always_thinking") return frozenset(caps) or None diff --git a/src/pythinker_code/auth/openai/models.py b/src/pythinker_code/auth/openai/models.py index cd117c8a..6150afdd 100644 --- a/src/pythinker_code/auth/openai/models.py +++ b/src/pythinker_code/auth/openai/models.py @@ -175,7 +175,7 @@ def _parse_chatgpt_models_payload(payload: object) -> list[ModelInfo]: raw_models = payload_object.get("models") if not isinstance(raw_models, list): # Keep a small compatibility path in case OpenAI ever aligns this with - # the public /v1/models shape. ChatGPT Codex currently returns + # the public /v1/models shape. The ChatGPT endpoint currently returns # {"models": [{"slug": ...}]}. raw_models = payload_object.get("data") if not isinstance(raw_models, list): diff --git a/src/pythinker_code/auth/openai/oauth_client.py b/src/pythinker_code/auth/openai/oauth_client.py index eaa199ba..7d88895f 100644 --- a/src/pythinker_code/auth/openai/oauth_client.py +++ b/src/pythinker_code/auth/openai/oauth_client.py @@ -184,7 +184,7 @@ def _token_from_openai_response(payload: dict[str, Any]) -> OAuthToken: # it lives inside the OAuth JWT claims under # `https://api.openai.com/auth.chatgpt_account_id`. Hoist it onto the # response so OAuthToken.from_response() picks it up. Without this the - # ChatGPT usage adapter, model catalog endpoint, and Codex request headers + # ChatGPT usage adapter, model catalog endpoint, and request headers # cannot scope requests to the active Plus/Pro account. if "account_id" not in normalized: jwt_token = payload.get("id_token") or payload.get("access_token") diff --git a/src/pythinker_code/auth/platforms.py b/src/pythinker_code/auth/platforms.py index e40bddcf..89a74931 100644 --- a/src/pythinker_code/auth/platforms.py +++ b/src/pythinker_code/auth/platforms.py @@ -176,7 +176,7 @@ def _select_retry_api_keys( def _openai_fallback_models(platform_id: str) -> list[ModelInfo] | None: - # ChatGPT Codex model availability is subscription/account-specific. Do not + # ChatGPT model availability is subscription/account-specific. Do not # replace the user's live catalog with a static fallback; stale fallback # slugs surface as 400 "model is not supported with a ChatGPT account". if platform_id == OPENAI_CHATGPT_PLATFORM_ID: diff --git a/src/pythinker_code/cli/__init__.py b/src/pythinker_code/cli/__init__.py index 336ca3b0..8e4ad092 100644 --- a/src/pythinker_code/cli/__init__.py +++ b/src/pythinker_code/cli/__init__.py @@ -911,7 +911,7 @@ async def _run(session_id: str | None, prefill_text: str | None = None) -> tuple scratchpad_status = await ensure_git_excluded(work_dir) # Sweep accumulated state on startup (best-effort, non-blocking). - # Mirrors Claude Code's cleanupPeriodDays=30 model. + # Default 30-day retention sweep. _retention = config.session_retention_days if isinstance(config, Config) else 30 await asyncio.to_thread(sweep_old_sessions, _retention) await asyncio.to_thread(sweep_old_plans, _retention) diff --git a/src/pythinker_code/hooks/runner.py b/src/pythinker_code/hooks/runner.py index 9d54d78c..bcdbd548 100644 --- a/src/pythinker_code/hooks/runner.py +++ b/src/pythinker_code/hooks/runner.py @@ -110,7 +110,7 @@ async def run_hook( def _extract_additional_context(parsed: dict[str, Any], hook_output: dict[str, Any]) -> str: - """Extract Claude-Code-style additionalContext from JSON hook output.""" + """Extract ``additionalContext`` from JSON hook output.""" candidates = (hook_output.get("additionalContext"), parsed.get("additionalContext")) for value in candidates: if isinstance(value, str) and value.strip(): diff --git a/src/pythinker_code/llm.py b/src/pythinker_code/llm.py index b9c9f235..7616f089 100644 --- a/src/pythinker_code/llm.py +++ b/src/pythinker_code/llm.py @@ -361,7 +361,7 @@ def create_llm( is_dashscope_legacy = provider.type == "openai_legacy" and _is_dashscope_endpoint( provider.base_url or "" ) - # Kimi K2.x uses the provider-specific thinking.type field on Moonshot-style + # Moonshot K2.x models use the provider-specific thinking.type field on Moonshot-style # endpoints, but Alibaba's DashScope-compatible routes use enable_thinking. is_kimi_openai_legacy = ( provider.type == "openai_legacy" @@ -381,7 +381,7 @@ def create_llm( # null reasoning_effort field. chat_provider = chat_provider.with_thinking(effective_effort) - # Kimi K2.x on Moonshot-style endpoints and GLM use thinking.type. + # Moonshot K2.x and GLM use thinking.type on Moonshot-style endpoints. if (is_kimi_openai_legacy or is_glm_openai_legacy) and effective_effort is not None: thinking_body: dict[str, object] = {"type": "enabled" if thinking_on else "disabled"} if is_glm_openai_legacy and thinking_on: @@ -466,14 +466,14 @@ def clone_llm_with_model_alias( def derive_model_capabilities(model: LLMModel) -> set[ModelCapability]: capabilities = set(model.capabilities or ()) model_name = model.model.lower() - # Kimi K2.5/K2.6 support thinking, but it can be disabled via + # Moonshot K2.5/K2.6 support thinking, but it can be disabled via # `thinking.type`. Keep them out of always_thinking so --no-thinking and the # default_thinking=false config path can send the provider-specific disable # switch in create_llm(). if _is_kimi_k2_model(model.model): capabilities.add("thinking") - # kimi-k2-thinking is Moonshot's thinking-only variant; unlike the - # hybrid K2.5/K2.6 it cannot be switched off. + # Moonshot's thinking-only K2 variant (its model name contains + # "thinking"); unlike the hybrid K2.5/K2.6 it cannot be switched off. if "thinking" in model_name: capabilities.add("always_thinking") # Models with "thinking" in their name are always-thinking models diff --git a/src/pythinker_code/session_cleanup.py b/src/pythinker_code/session_cleanup.py index dd67f98d..a95d450b 100644 --- a/src/pythinker_code/session_cleanup.py +++ b/src/pythinker_code/session_cleanup.py @@ -1,9 +1,8 @@ """Age-based cleanup for personal-scope session and plan state. -Runs at agent startup to keep ~/.pythinker/ from growing unboundedly. -Mirrors the design used by Claude Code (cleanupPeriodDays=30): on startup, -directories/files older than the retention threshold are removed if they are -safe to discard (archived sessions, old plan files). +Runs at agent startup to keep ~/.pythinker/ from growing unboundedly: with a +30-day retention period, directories/files older than the retention threshold +are removed if they are safe to discard (archived sessions, old plan files). Never raises — every error is logged at DEBUG level and silently skipped. """ diff --git a/src/pythinker_code/soul/dynamic_injections/plan_mode.py b/src/pythinker_code/soul/dynamic_injections/plan_mode.py index f9af92dd..c31213a8 100644 --- a/src/pythinker_code/soul/dynamic_injections/plan_mode.py +++ b/src/pythinker_code/soul/dynamic_injections/plan_mode.py @@ -32,6 +32,12 @@ async def get_injections( history: Sequence[Message], soul: PythinkerSoul, ) -> list[DynamicInjection]: + # Plan-mode workflow reminders are root-only. Subagents share the + # session's plan_mode flag (so persistence/resume work), but their YAMLs + # usually exclude EnterPlanMode/ExitPlanMode, so do not inject this + # workflow guidance into subagent contexts. + if soul.is_subagent: + return [] if not soul.plan_mode: self._inject_count = 0 return [] diff --git a/src/pythinker_code/soul/pythinkersoul.py b/src/pythinker_code/soul/pythinkersoul.py index 7ec25df3..e79c3a1f 100644 --- a/src/pythinker_code/soul/pythinkersoul.py +++ b/src/pythinker_code/soul/pythinkersoul.py @@ -2174,7 +2174,7 @@ def _is_retryable_error(exception: BaseException) -> bool: if not isinstance(exception, APIStatusError): return False if exception.status_code == 429 and _is_hard_usage_limit(exception): - # A subscription usage cap (e.g. ChatGPT Codex `usage_limit_reached`) + # A subscription usage cap (e.g. ChatGPT `usage_limit_reached`) # resets in hours, not seconds — retrying with backoff only adds # latency before the inevitable failure. Surface it immediately. return False diff --git a/src/pythinker_code/tools/shell/__init__.py b/src/pythinker_code/tools/shell/__init__.py index 4c3d7756..5b1e8664 100644 --- a/src/pythinker_code/tools/shell/__init__.py +++ b/src/pythinker_code/tools/shell/__init__.py @@ -176,9 +176,13 @@ def stderr_cb(line: bytes): return builder.ok("Command executed successfully.", status=ToolResultStatus.success) builder.extras(exit_code=exitcode) + brief = f"Failed with exit code: {exitcode}" + tail = builder.tail() + if tail: + brief += f"\n{tail}" return builder.error( f"Command failed with exit code: {exitcode}.", - brief=f"Failed with exit code: {exitcode}", + brief=brief, status=ToolResultStatus.failure, ) except TimeoutError: diff --git a/src/pythinker_code/tools/utils.py b/src/pythinker_code/tools/utils.py index 0b3408b7..70fd7eec 100644 --- a/src/pythinker_code/tools/utils.py +++ b/src/pythinker_code/tools/utils.py @@ -213,6 +213,26 @@ def write(self, text: str) -> int: return chars_written + def tail(self, max_lines: int = 5, max_line_len: int = 200) -> str: + """Return the last non-empty lines from the buffer, joined with newlines. + + Useful for surfacing actionable error context (stderr) in tool result briefs. + """ + collected: list[str] = [] + for chunk in reversed(self._buffer): + for line in reversed(chunk.splitlines()): + stripped = line.rstrip() + if not stripped.strip(): + continue + if len(stripped) > max_line_len: + stripped = stripped[:max_line_len] + "..." + collected.append(stripped) + if len(collected) >= max_lines: + break + if len(collected) >= max_lines: + break + return "\n".join(reversed(collected)) + def display(self, *blocks: DisplayBlock) -> None: """Add display blocks to the tool result.""" self._display.extend(blocks) diff --git a/src/pythinker_code/ui/color_utils.py b/src/pythinker_code/ui/color_utils.py index 34c21fa5..bf874630 100644 --- a/src/pythinker_code/ui/color_utils.py +++ b/src/pythinker_code/ui/color_utils.py @@ -1,7 +1,6 @@ """Small color-math helpers for terminal-adaptive UI decisions. -Ported from the Codex TUI reference (``codex-rs/tui/src/color.rs``): linear -RGB blending plus an ITU-R BT.601 luma test used to classify terminal +Linear RGB blending plus an ITU-R BT.601 luma test used to classify terminal backgrounds as light or dark. Pure functions, no terminal I/O. """ diff --git a/src/pythinker_code/ui/shell/__init__.py b/src/pythinker_code/ui/shell/__init__.py index 7946a219..d13b4c2e 100644 --- a/src/pythinker_code/ui/shell/__init__.py +++ b/src/pythinker_code/ui/shell/__init__.py @@ -25,6 +25,7 @@ ChatProviderError, ) from rich import box +from rich.align import Align from rich.console import Group, RenderableType from rich.markup import escape from rich.panel import Panel @@ -77,6 +78,7 @@ ApprovalPromptDelegate, visualize, ) +from pythinker_code.ui.terminal_capabilities import ascii_glyphs_enabled, motion_disabled from pythinker_code.ui.theme import get_tui_tokens as _get_tui_tokens from pythinker_code.ui.theme import tui_rich_style from pythinker_code.utils.aioqueue import QueueShutDown @@ -824,6 +826,12 @@ async def run(self, command: str | None = None) -> bool: else: self._start_background_task(self._auto_update()) + if isinstance(self.soul, PythinkerSoul): + # Kick off MCP loading before the banner so servers connect in the + # background while the user reads it; the prompt's MCP status line + # carries the blinking "connecting" heartbeat without ever + # blocking input. + await self.soul.start_background_mcp_loading() _print_welcome_info( self.soul.name or "Pythinker CLI", self._welcome_info, @@ -856,7 +864,6 @@ async def run(self, command: str | None = None) -> bool: wire_file=self.soul.wire_file, show_thinking_stream=self.soul.runtime.config.show_thinking_stream, ) - await self.soul.start_background_mcp_loading() async def _plan_mode_toggle() -> bool: if isinstance(self.soul, PythinkerSoul): @@ -2091,10 +2098,15 @@ def _cancel_background_tasks(self) -> None: _LOGO_NAVY = "#213853" # outline / chassis (head + body frame, mouth, neck) _LOGO_FACE = "#F9F2F5" # face / chest interior (cream) _LOGO_CORAL = "#EE9983" # antenna ball, ears, accent bits +_LOGO_CORAL_LIT = "#FFB9A3" # antenna ball "powered on" — lighter coral glow _LOGO_IRIS = "#AFE3F1" # eye iris + chest button glow (brand cyan) -_LOGO = ( - f" [{_LOGO_CORAL}]●[/]\n" +# Head-only robot mark (antenna, ears, eyes, mouth). Only rendered when +# ascii_glyphs_enabled() is false; ASCII terminals get the text-only banner +# instead of a garbled silhouette. ``{antenna_style}`` is filled per render so +# the antenna ball can carry the terminal's slow-blink attribute. +_LOGO_TEMPLATE = ( + " [{antenna_style}]●[/]\n" f" [{_LOGO_NAVY}]│[/]\n" f" [{_LOGO_NAVY}]▛[/][{_LOGO_FACE}]▀▀▀▀▀▀▀[/][{_LOGO_NAVY}]▜[/]\n" f" [{_LOGO_CORAL}]◖[/][{_LOGO_NAVY}]█[/][{_LOGO_FACE}] [/]" @@ -2104,6 +2116,35 @@ def _cancel_background_tasks(self) -> None: ) +def _logo_text() -> Text: + """Robot mark with a glowing antenna ball. + + The ball carries the terminal's SGR slow-blink attribute, so terminals + with blinking text enabled blink it indefinitely; everywhere else the + bold light-coral glow reads as "powered on". Reduced motion pins the + ball steady and muted. + """ + antenna_style = _LOGO_CORAL if motion_disabled() else f"blink bold {_LOGO_CORAL_LIT}" + return Text.from_markup(_LOGO_TEMPLATE.format(antenna_style=antenna_style)) + + +# 1:1 ASCII stand-ins for every decorative glyph the welcome banner emits +# (mirrors the server-banner fallback in utils/server.py). Welcome copy and +# chips pass through this when ascii_glyphs_enabled() is true so legacy code +# pages never see a raw Unicode glyph in the startup path. +_WELCOME_ASCII_FALLBACKS = str.maketrans( + { + "✦": "*", + "↑": "^", + "•": "*", + "·": "-", + "—": "-", + "…": "~", + "─": "-", + } +) + + @dataclass(slots=True) class WelcomeInfoItem: class Level(Enum): @@ -2131,7 +2172,7 @@ def _value_style_for_label(label: str, level: WelcomeInfoItem.Level) -> str: if label == "Model": return f"bold {tokens.text}" if tokens.text else "bold bright_white" if label == "Branch": - return tokens.info or "cyan" + return tokens.muted or "grey50" if label == "Auto-save": return tokens.muted or "grey50" return level.value @@ -2148,22 +2189,37 @@ def _welcome_banner_chip() -> Text | None: whats_new_version = consume_whats_new() update_target = welcome_update_target() - if update_target: - chip = Text.from_markup(f"[{_t.warning}]↑ Update available — v{update_target} · /update[/]") - chip.highlight_regex(r"/[A-Za-z][A-Za-z0-9_-]*", f"bold {_t.warning}") + def _chip(markup: str, style: str) -> Text: + if ascii_glyphs_enabled(): + markup = markup.translate(_WELCOME_ASCII_FALLBACKS) + chip = Text.from_markup(markup) + chip.highlight_regex(r"/[A-Za-z][A-Za-z0-9_-]*", f"bold {style}") return chip + if update_target: + return _chip( + f"[{_t.warning}]↑ Update available — v{update_target} · /update[/]", _t.warning + ) + if whats_new_version: - chip = Text.from_markup(f"[{_t.info}]✦ What's new in v{whats_new_version} · /changelog[/]") - chip.highlight_regex(r"/[A-Za-z][A-Za-z0-9_-]*", f"bold {_t.info}") - return chip + return _chip(f"[{_t.info}]✦ What's new in v{whats_new_version} · /changelog[/]", _t.info) return None -_WELCOME_MAX_WIDTH = 100 _WELCOME_LABEL_WIDTH = 10 _WELCOME_PANEL_CHROME_WIDTH = 6 # border + horizontal padding used below +# Content cells needed before tips move into a divided right-hand column +# (welcome copy + robot on the left, tips on the right, facts full-width +# below). Narrower terminals stack the same blocks vertically instead. +_WELCOME_COLUMNS_MIN_WIDTH = 84 +# Two-column proportions: the left column stays narrow — wide enough +# for the strapline (52 cells), growing up to the max to fit fact rows — +# and tips absorb the remaining width. +_WELCOME_LEFT_COLUMN_WIDTH = 52 +_WELCOME_LEFT_COLUMN_MAX_WIDTH = 64 +_WELCOME_TIPS_MIN_WIDTH = 24 +_WELCOME_COLUMNS_CHROME_WIDTH = 3 # divider + one pad cell each side def _take_cells_left(text: str, max_width: int) -> str: @@ -2194,7 +2250,7 @@ def _take_cells_right(text: str, max_width: int) -> str: return "".join(reversed(out)) -def _truncate_middle_to_width(text: str, max_width: int) -> str: +def _truncate_middle_to_width(text: str, max_width: int, *, ellipsis: str = "…") -> str: """Cell-aware middle truncation for paths and UUID-like values.""" if max_width <= 0: return "" @@ -2202,25 +2258,29 @@ def _truncate_middle_to_width(text: str, max_width: int) -> str: if cell_width(cleaned) <= max_width: return cleaned if max_width <= 1: - return truncate_to_width(cleaned, max_width) + return truncate_to_width(cleaned, max_width, ellipsis=ellipsis) left_width = max(1, (max_width - 1) // 2) right_width = max(1, max_width - 1 - left_width) - return f"{_take_cells_left(cleaned, left_width)}…{_take_cells_right(cleaned, right_width)}" + return ( + f"{_take_cells_left(cleaned, left_width)}{ellipsis}" + f"{_take_cells_right(cleaned, right_width)}" + ) def _welcome_panel_width() -> int: - columns = current_console_width(console, default=_WELCOME_MAX_WIDTH) - return max(1, min(columns, _WELCOME_MAX_WIDTH)) + # Span the full terminal, matching the prompt/input rules below the + # banner; re-queried at print time so every tab/terminal gets its own fit. + return max(1, current_console_width(console, default=80)) -def _welcome_value(label: str, value: str, max_width: int) -> str: +def _welcome_value(label: str, value: str, max_width: int, *, ellipsis: str = "…") -> str: cleaned = sanitize_ansi(value).replace("\r", " ").replace("\n", " ") if label.strip() in {"Directory", "Auto-save", "Session"}: - return _truncate_middle_to_width(cleaned, max_width) - return truncate_to_width(cleaned, max_width) + return _truncate_middle_to_width(cleaned, max_width, ellipsis=ellipsis) + return truncate_to_width(cleaned, max_width, ellipsis=ellipsis) -def _welcome_tip_lines(value: str, max_width: int) -> list[str]: +def _welcome_tip_lines(value: str, max_width: int, *, ellipsis: str = "…") -> list[str]: cleaned = sanitize_ansi(value).replace("\r", " ").replace("\n", " ").strip() if not cleaned: return [""] @@ -2230,43 +2290,48 @@ def _welcome_tip_lines(value: str, max_width: int) -> list[str]: break_long_words=False, break_on_hyphens=False, ) or [cleaned] - return [truncate_to_width(line, max_width) for line in lines] + return [truncate_to_width(line, max_width, ellipsis=ellipsis) for line in lines] def _print_welcome_info( name: str, info_items: list[WelcomeInfoItem], *, banner: Text | None = None ) -> None: + """Print the welcome banner once; it must never block the prompt.""" _t = _get_tui_tokens() + ascii_mode = ascii_glyphs_enabled() + ellipsis = "~" if ascii_mode else "…" + panel_box = box.ASCII if ascii_mode else box.ROUNDED panel_width = _welcome_panel_width() content_width = max(1, panel_width - _WELCOME_PANEL_CHROME_WIDTH) - head = Text.from_markup("[bold]Welcome to Pythinker — think first, then code.[/]") - strapline = Text.from_markup( - f"[{_t.muted}]Review · Secure · Diagnose · Build with confidence.[/]" - ) - help_text = Text.from_markup(f"[{_t.muted}]Type /help for commands.[/]") + def _copy(markup: str) -> Text: + if ascii_mode: + markup = markup.translate(_WELCOME_ASCII_FALLBACKS) + return Text.from_markup(markup) + + head = _copy("[bold]Welcome to Pythinker — think first, then code.[/]") + strapline = _copy(f"[{_t.muted}]Review · Secure · Diagnose · Build with confidence.[/]") + help_text = _copy(f"[{_t.muted}]Type /help for commands.[/]") help_text.highlight_regex(r"/help\b", f"bold {_LOGO_CORAL}") - rows: list[RenderableType] = [] - if content_width >= 68: - # Logo on the left; the 3-line text block bottom-aligns against the 5-line - # robot so the antenna floats above and the lines sit beside the body. - logo = Text.from_markup(_LOGO) - table = Table.grid(padding=(0, 1)) - table.add_column(justify="left", no_wrap=True) - table.add_column(justify="left", vertical="bottom", no_wrap=True) - table.add_row(logo, Group(head, strapline, help_text)) - rows.append(table) - else: - rows.extend([head, strapline, help_text]) + if ascii_mode: + # Caller-provided values (tips, notices) may carry the same decorative + # glyphs as our own copy; degrade them through the same table. + info_items = [ + WelcomeInfoItem( + name=item.name, + value=item.value.translate(_WELCOME_ASCII_FALLBACKS), + level=item.level, + ) + for item in info_items + ] facts = [item for item in info_items if item.name.strip() != "Tip"] tips = [item for item in info_items if item.name.strip() == "Tip"] - if facts: - rows.append(Text("")) # empty line - label_width = min(_WELCOME_LABEL_WIDTH, max(4, content_width // 3)) - value_width = max(4, content_width - label_width - 2) + def _facts_grid(width: int) -> Table: + label_width = min(_WELCOME_LABEL_WIDTH, max(4, width // 3)) + value_width = max(4, width - label_width - 2) info_table = Table.grid(padding=(0, 1)) info_table.add_column( justify="right", @@ -2277,40 +2342,112 @@ def _print_welcome_info( info_table.add_column(justify="left", no_wrap=True, width=value_width) for item in facts: value_style = _value_style_for_label(item.name, item.level) - value = _welcome_value(item.name, item.value, value_width) + value = _welcome_value(item.name, item.value, value_width, ellipsis=ellipsis) info_table.add_row(item.name, Text(value, style=value_style, no_wrap=True)) - rows.append(info_table) - - if tips: - rows.append(Text("")) # empty line - rows.append(Text("Tips", style=tui_rich_style("muted"))) - tip_width = max(4, content_width - 4) + return info_table + + def _tips_block(width: int, *, with_rule: bool) -> Group: + gutter = 2 + tip_width = max(4, width - gutter) + parts: list[RenderableType] = [Text("Tips", style=tui_rich_style("muted"))] + if with_rule: + rule_char = "-" if ascii_mode else "─" + parts.append(Text(rule_char * max(4, width), style=tui_rich_style("muted"))) tips_table = Table.grid(padding=(0, 0)) - tips_table.add_column(style=tui_rich_style("muted"), no_wrap=True, width=4) + tips_table.add_column(style=tui_rich_style("muted"), no_wrap=True, width=gutter) tips_table.add_column(justify="left", no_wrap=True, width=tip_width) + bullet = "* " if ascii_mode else "• " for item in tips: - for index, line in enumerate(_welcome_tip_lines(item.value, tip_width)): + lines = _welcome_tip_lines(item.value, tip_width, ellipsis=ellipsis) + for index, line in enumerate(lines): tip_text = Text(line, style=item.level.value, no_wrap=True) tip_text.highlight_regex(r"/[A-Za-z][A-Za-z0-9_-]*", f"bold {_LOGO_CORAL}") - tips_table.add_row(" • " if index == 0 else " ", tip_text) - rows.append(tips_table) + tips_table.add_row(bullet if index == 0 else " ", tip_text) + parts.append(tips_table) + return Group(*parts) + + show_logo = not ascii_mode + use_columns = bool(tips) and content_width >= _WELCOME_COLUMNS_MIN_WIDTH + logo_rendered = show_logo and (use_columns or content_width >= 68) version_title = Text.assemble( ("Pythinker Code", tui_rich_style("muted")), (f" v{get_version()}", tui_rich_style("dim")), ) - console.print( - Panel( + def _panel() -> Panel: + rows: list[RenderableType] = [] + if use_columns: + # Two-column split: welcome copy, the robot mark, and the fact + # rows on the left; tips in a divided right-hand column. The left + # column grows (within its cap) to fit the longest fact row so + # paths don't truncate while the tips column has slack. + wanted_left = _WELCOME_LEFT_COLUMN_WIDTH + if facts: + longest_fact = max(cell_width(item.value) for item in facts) + wanted_left = max( + wanted_left, + min(_WELCOME_LEFT_COLUMN_MAX_WIDTH, longest_fact + _WELCOME_LABEL_WIDTH + 2), + ) + left_width = max( + _WELCOME_LEFT_COLUMN_WIDTH, + min( + wanted_left, + content_width - _WELCOME_COLUMNS_CHROME_WIDTH - _WELCOME_TIPS_MIN_WIDTH, + ), + ) + tips_width = content_width - _WELCOME_COLUMNS_CHROME_WIDTH - left_width + left_rows: list[RenderableType] = [head, strapline, help_text] + if show_logo: + left_rows.extend([Text(""), Align.center(_logo_text())]) + if facts: + left_rows.extend([Text(""), _facts_grid(left_width)]) + columns = Table( + box=panel_box, + show_header=False, + show_edge=False, + show_lines=False, + pad_edge=False, + padding=(0, 1), + border_style=tui_rich_style("border"), + expand=False, + ) + columns.add_column(width=left_width, justify="left", vertical="top") + columns.add_column(width=tips_width, justify="left", vertical="top") + columns.add_row(Group(*left_rows), _tips_block(tips_width, with_rule=True)) + rows.append(columns) + else: + if logo_rendered: + # Logo on the left; the text block centers vertically against + # the robot so the lines sit beside the face while the antenna + # floats above. + table = Table.grid(padding=(0, 1)) + table.add_column(justify="left", no_wrap=True) + table.add_column(justify="left", vertical="middle", no_wrap=True) + table.add_row(_logo_text(), Group(head, strapline, help_text)) + rows.append(table) + else: + rows.extend([head, strapline, help_text]) + + if facts: + rows.append(Text("")) # empty line + rows.append(_facts_grid(content_width)) + + if tips: + rows.append(Text("")) # empty line + rows.append(_tips_block(content_width, with_rule=False)) + + return Panel( Group(*rows), title=version_title, title_align="left", subtitle=banner, subtitle_align="right", border_style=tui_rich_style("border"), - box=box.ROUNDED, + box=panel_box, expand=False, width=panel_width, padding=(1, 2), ) - ) + + console.print(_panel()) diff --git a/src/pythinker_code/ui/shell/components/markdown.py b/src/pythinker_code/ui/shell/components/markdown.py index ca5e8444..54f0f060 100644 --- a/src/pythinker_code/ui/shell/components/markdown.py +++ b/src/pythinker_code/ui/shell/components/markdown.py @@ -438,7 +438,7 @@ def __rich_console__(self, console: Console, options: ConsoleOptions) -> RenderR lexer_name = self.lexer_name.strip() title = lexer_name if lexer_name and lexer_name != "text" else None - # Size guard (Codex parity): skip Pygments for very large blocks so a + # Size guard: skip Pygments for very large blocks so a # pathological fence cannot stall the renderer. ``len()`` counts # characters (a lower bound on UTF-8 bytes), which is enough for a # guard heuristic without paying for an encode of the whole block. diff --git a/src/pythinker_code/ui/shell/mcp_status.py b/src/pythinker_code/ui/shell/mcp_status.py index bf9bee63..2270a400 100644 --- a/src/pythinker_code/ui/shell/mcp_status.py +++ b/src/pythinker_code/ui/shell/mcp_status.py @@ -10,11 +10,21 @@ from pythinker_code.ui.shell.components.render_utils import sanitize_ansi from pythinker_code.ui.shell.glyphs import LIST_BULLET, TRANSCRIPT_ACTIVE_MARKER from pythinker_code.ui.shell.motion import blink_visible +from pythinker_code.ui.terminal_capabilities import colors_disabled from pythinker_code.ui.theme import get_mcp_prompt_colors, tui_rich_style from pythinker_code.wire.types import MCPServerSnapshot, MCPStatusSnapshot _STARTING_STATUSES = frozenset({"pending", "connecting"}) +# The blinking startup marker doubles as the robot's heartbeat: it carries the +# welcome banner antenna ball's coral (see _LOGO_CORAL in ui/shell/__init__.py; +# kept literal here because mcp_status is imported by that package). +_HEARTBEAT_CORAL = "#EE9983" + + +def _heartbeat_rich_style() -> Style: + return Style() if colors_disabled() else Style(color=_HEARTBEAT_CORAL, bold=True) + def _safe_text(text: str) -> str: return sanitize_ansi(text).replace("\r\n", " ").replace("\r", " ").replace("\n", " ") @@ -57,7 +67,7 @@ def render_mcp_startup_text(snapshot: MCPStatusSnapshot, *, now: float | None = """Render the animated MCP startup status used by live prompt/status areas.""" t = time.monotonic() if now is None else now glyph = TRANSCRIPT_ACTIVE_MARKER if blink_visible(t) else " " - line = Text(f"{glyph} ", style=tui_rich_style("muted")) + line = Text(f"{glyph} ", style=_heartbeat_rich_style()) line.append( mcp_startup_header(snapshot) or "Starting MCP servers", style=tui_rich_style("muted"), @@ -102,7 +112,7 @@ def render_mcp_console(snapshot: MCPStatusSnapshot) -> RenderableType: def render_mcp_inventory_loading(*, now: float | None = None) -> RenderableType: t = time.monotonic() if now is None else now glyph = TRANSCRIPT_ACTIVE_MARKER if blink_visible(t) else " " - line = Text(f"{glyph} ", style=tui_rich_style("muted")) + line = Text(f"{glyph} ", style=_heartbeat_rich_style()) line.append("Loading MCP inventory", style=tui_rich_style("tool_title") + Style(bold=True)) line.append("…", style=tui_rich_style("muted")) return line @@ -140,8 +150,8 @@ def render_mcp_prompt(snapshot: MCPStatusSnapshot, *, now: float | None = None) colors = get_mcp_prompt_colors() t = time.monotonic() if now is None else now glyph = TRANSCRIPT_ACTIVE_MARKER if blink_visible(t) else " " - prefix = f"{glyph} " - return FormattedText([(colors.text, f"{prefix}{header}"), ("", "\n")]) + prefix_style = colors.text if colors_disabled() else f"bold {_HEARTBEAT_CORAL}" + return FormattedText([(prefix_style, f"{glyph} "), (colors.text, header), ("", "\n")]) def _status_color(status: str) -> Style: diff --git a/src/pythinker_code/ui/shell/motion.py b/src/pythinker_code/ui/shell/motion.py index 3ee56dbc..d5586ca0 100644 --- a/src/pythinker_code/ui/shell/motion.py +++ b/src/pythinker_code/ui/shell/motion.py @@ -97,7 +97,7 @@ def _wave_colors( trail behind it — an angled sheen rather than a flat pulse. ``rightward`` flips both the travel direction and the trailing side so the trail always lags behind the head. With ``smooth`` (truecolor terminals), the sheen is a - continuous cosine-falloff blend from highlight into base (Codex shimmer) + continuous cosine-falloff blend from highlight into base instead of the discrete three-step ramp. """ n = len(chars) diff --git a/src/pythinker_code/ui/shell/prompt.py b/src/pythinker_code/ui/shell/prompt.py index 72b72e52..b9924891 100644 --- a/src/pythinker_code/ui/shell/prompt.py +++ b/src/pythinker_code/ui/shell/prompt.py @@ -865,8 +865,8 @@ def _render_command_text( if match_prefix_len <= 0: return FormattedText([(base_style, display)]) - # Match highlighting mirrors Codex's slash popup: the leading slash stays - # in the normal command style; the typed command prefix is emphasized. + # Match highlighting for the slash popup: the leading slash stays in the + # normal command style; the typed command prefix is emphasized. match_end = min(len(text), 1 + match_prefix_len) match_style = ( "class:slash-completion-menu.command.match.current" @@ -1258,7 +1258,7 @@ def _get_deep_paths(self) -> list[str]: now - self._cache_time <= self._refresh_interval and self._cache_scope == scope ) - # Invalidate on .git/index mtime change (like Claude Code). + # Invalidate on .git/index mtime change. if cache_valid and self._is_git: mtime = git_index_mtime(self._root) if mtime != self._git_index_mtime: @@ -3563,8 +3563,8 @@ def _append_right(style: str, text: str) -> None: right_width = _display_width(right_text) # Left side: prefer extension statuses, then active background work, - # then any active toast. The background-work copy mirrors Codex's - # compact footer summary while keeping Pythinker's single /task command. + # then any active toast. The background-work copy is a compact footer + # summary using Pythinker's single /task command. max_left_width = max(0, columns - right_width - 2) ext = footer_statuses() if ext: diff --git a/src/pythinker_code/ui/shell/render_constants.py b/src/pythinker_code/ui/shell/render_constants.py index 799b88ec..2239bdf3 100644 --- a/src/pythinker_code/ui/shell/render_constants.py +++ b/src/pythinker_code/ui/shell/render_constants.py @@ -31,7 +31,7 @@ #: pathological diff cannot freeze or flood the terminal. DIFF_EXPANDED_MAX_LINES: Final = 400 -#: Syntax-highlighting size guards (Codex parity: render/highlight.rs). Code +#: Syntax-highlighting size guards. Code #: blocks beyond either limit render as plain text with a notice instead of #: paying an unbounded Pygments lexing cost. MAX_HIGHLIGHT_BYTES: Final = 512 * 1024 diff --git a/src/pythinker_code/ui/shell/slash.py b/src/pythinker_code/ui/shell/slash.py index 9f02686b..d1894678 100644 --- a/src/pythinker_code/ui/shell/slash.py +++ b/src/pythinker_code/ui/shell/slash.py @@ -1649,8 +1649,8 @@ def web(app: Shell, args: str): @registry.command -def vis(app: Shell, args: str): - """Open Pythinker Agent Tracing Visualizer in browser""" +def reports(app: Shell, args: str): + """Open Pythinker session reports (Agent Tracing Visualizer) in browser""" from pythinker_code.telemetry import track track("vis_opened") diff --git a/src/pythinker_code/ui/shell/stats_pricing.py b/src/pythinker_code/ui/shell/stats_pricing.py index 8079845e..3f47b353 100644 --- a/src/pythinker_code/ui/shell/stats_pricing.py +++ b/src/pythinker_code/ui/shell/stats_pricing.py @@ -51,7 +51,7 @@ "glm-5-turbo": (0.5, 1.5, 0.1, 0.0), "glm-4.7": (0.5, 1.5, 0.1, 0.0), "glm-4.5-air": (0.3, 1.0, 0.06, 0.0), - # Kimi (opencode-go) + # Moonshot K2 (opencode-go) "kimi-k2.5": (0.6, 3.0, 0.08, 0.0), "kimi-k2.6": (0.95, 4.0, 0.16, 0.0), # MiniMax (opencode-go / anthropic shape) diff --git a/src/pythinker_code/ui/shell/usage_adapters/openai_chatgpt.py b/src/pythinker_code/ui/shell/usage_adapters/openai_chatgpt.py index 2b5fbf37..76b23a84 100644 --- a/src/pythinker_code/ui/shell/usage_adapters/openai_chatgpt.py +++ b/src/pythinker_code/ui/shell/usage_adapters/openai_chatgpt.py @@ -117,7 +117,7 @@ def parse_codex_usage_payload(payload: object) -> UsageReport: elif rate_map.get("allowed") is False: notes.append("Requests currently not allowed by the rate limiter.") - # Codex's wham/usage response carries up to two windows under + # The ChatGPT wham/usage response carries up to two windows under # `primary_window` / `secondary_window` (older releases used `five_hour` / # `weekly`). Accounts on some plans only return one of them — and the slot # name is NOT a reliable indicator of which window is shorter (e.g. free diff --git a/src/pythinker_code/ui/shell/visualize/_worklog.py b/src/pythinker_code/ui/shell/visualize/_worklog.py index 9310255c..914fb9cf 100644 --- a/src/pythinker_code/ui/shell/visualize/_worklog.py +++ b/src/pythinker_code/ui/shell/visualize/_worklog.py @@ -226,11 +226,18 @@ def render_display_blocks( if text: title = "Error" if is_error else "Report" style = tui_rich_style("error") if is_error else tui_rich_style("muted") + # Error briefs carry a trailing stderr tail (the failing command's + # last lines). Rendering that as Markdown reflows newlines and eats + # shell metacharacters (backticks, '#', '*'), so render errors as + # plain text and reserve Markdown for authored "Report" briefs. + body: RenderableType = ( + Text(text, style=style) if is_error else Markdown(text, style=style) + ) if "\n" in text or len(text) > 100: rendered.append( render_worklog_card( title, - Markdown(text, style=style), + body, border_style=tui_rich_style("error") if is_error else tui_rich_style("dim"), @@ -238,7 +245,7 @@ def render_display_blocks( ) ) else: - rendered.append(Markdown(text, style=style)) + rendered.append(body) idx += 1 continue if isinstance(block, TodoDisplayBlock): diff --git a/src/pythinker_code/ui/terminal_background.py b/src/pythinker_code/ui/terminal_background.py index 5d3573a6..3daba048 100644 --- a/src/pythinker_code/ui/terminal_background.py +++ b/src/pythinker_code/ui/terminal_background.py @@ -1,8 +1,7 @@ """Terminal default-background probing for ``theme = "auto"``. -Ported from the Codex TUI reference (``codex-rs/tui/src/terminal_probe.rs`` / -``terminal_palette.rs``): query the terminal's default background color with -OSC 11, classify it as light or dark via BT.601 luma, and cache the answer +Queries the terminal's default background color with +OSC 11, classifies it as light or dark via BT.601 luma, and caches the answer for the process lifetime. Probing is strictly best-effort — any failure (non-tty, Windows, dumb terminal, timeout, unparsable reply) returns ``None`` so callers keep their configured fallback. diff --git a/src/pythinker_code/ui/terminal_capabilities.py b/src/pythinker_code/ui/terminal_capabilities.py index 315b7c6a..7d051ed6 100644 --- a/src/pythinker_code/ui/terminal_capabilities.py +++ b/src/pythinker_code/ui/terminal_capabilities.py @@ -53,7 +53,7 @@ def colors_disabled(environ: Mapping[str, str] | None = None) -> bool: def color_depth(environ: Mapping[str, str] | None = None) -> ColorDepth: """Classify the terminal's color support into three usable tiers. - Mirrors the Codex TUI detection order: an explicit ``FORCE_COLOR`` level + Detection order: an explicit ``FORCE_COLOR`` level wins, then ``COLORTERM`` truecolor advertising, then the Windows Terminal promotion (``WT_SESSION`` implies 24-bit support even when ``TERM`` is conservative), then ``TERM`` itself. ``"none"`` mirrors diff --git a/src/pythinker_code/ui/theme.py b/src/pythinker_code/ui/theme.py index bfe980a6..439bc6e9 100644 --- a/src/pythinker_code/ui/theme.py +++ b/src/pythinker_code/ui/theme.py @@ -83,8 +83,8 @@ class DiffColors: ) # Basic 16-color terminals: the hex background tints above quantize into -# unreadable mud, so fall back to plain green/red foregrounds (Codex's -# ANSI16 diff tier). The fields still act as overlay styles for diff rows. +# unreadable mud, so fall back to plain green/red foregrounds (the ANSI16 +# diff tier). The fields still act as overlay styles for diff rows. _DIFF_ANSI16 = DiffColors( add_bg=RichStyle(color="green"), del_bg=RichStyle(color="red"), diff --git a/src/pythinker_code/utils/path.py b/src/pythinker_code/utils/path.py index 39744f6e..a5771cd5 100644 --- a/src/pythinker_code/utils/path.py +++ b/src/pythinker_code/utils/path.py @@ -174,7 +174,7 @@ def is_config_surface_path(path: HostPath, work_dir: HostPath | None = None) -> return is_within_directory(work_dir, agents_dir) or is_within_directory(path, work_dir) if "/.pythinker/" in posix_lower and base in ("config.toml", "config.local.toml"): return True - # Agent-spec dirs hold both YAML wrappers and Claude/Agents-style ``*.md`` + # Agent-spec dirs hold both YAML wrappers and Markdown-style ``*.md`` # frontmatter specs (see ``discover_markdown_agents``); both define a subagent's # tool policy and system prompt, so both are config surfaces. return base.endswith((".yaml", ".yml", ".md")) and any( diff --git a/src/pythinker_code/utils/term.py b/src/pythinker_code/utils/term.py index 7baacb06..b7623bda 100644 --- a/src/pythinker_code/utils/term.py +++ b/src/pythinker_code/utils/term.py @@ -75,6 +75,10 @@ def ensure_tty_sane() -> None: return attrs[3] |= desired + # Reset VMIN/VTIME to canonical defaults so the terminal is not left + # in cbreak mode after a hang or crash in _cursor_position_unix(). + attrs[6][termios.VMIN] = 1 + attrs[6][termios.VTIME] = 0 with contextlib.suppress(OSError): termios.tcsetattr(fd, termios.TCSADRAIN, attrs) @@ -92,9 +96,15 @@ def _cursor_position_unix() -> tuple[int, int] | None: fd = sys.stdin.fileno() oldterm = termios.tcgetattr(fd) + was_blocking = True try: tty.setcbreak(fd) + # Make reads non-blocking so that asyncio cancellation (or a race + # with prompt_toolkit's own stdin reader) cannot leave us stuck in + # an uninterruptible os.read() syscall. + was_blocking = os.get_blocking(fd) + os.set_blocking(fd, False) sys.stdout.write(_CURSOR_QUERY) sys.stdout.flush() @@ -107,6 +117,8 @@ def _cursor_position_unix() -> tuple[int, int] | None: continue try: chunk = os.read(fd, 32) + except BlockingIOError: + continue except OSError: break if not chunk: @@ -116,6 +128,8 @@ def _cursor_position_unix() -> tuple[int, int] | None: if match: return int(match.group(1)), int(match.group(2)) finally: + with contextlib.suppress(OSError): + os.set_blocking(fd, was_blocking) termios.tcsetattr(fd, termios.TCSADRAIN, oldterm) return None diff --git a/tests/core/test_plan_mode_injection_provider.py b/tests/core/test_plan_mode_injection_provider.py index 00c3d80f..585b867c 100644 --- a/tests/core/test_plan_mode_injection_provider.py +++ b/tests/core/test_plan_mode_injection_provider.py @@ -19,9 +19,11 @@ def _make_soul_mock( plan_mode: bool = True, plan_path: Path | None = None, consume_pending: bool = False, + is_subagent: bool = False, ) -> MagicMock: soul = MagicMock() type(soul).plan_mode = PropertyMock(return_value=plan_mode) + type(soul).is_subagent = PropertyMock(return_value=is_subagent) soul.get_plan_file_path.return_value = plan_path soul.consume_pending_plan_activation_injection.return_value = consume_pending return soul @@ -134,6 +136,22 @@ async def test_resets_count_when_deactivated(self) -> None: await provider.get_injections([], soul) assert provider._inject_count == 0 + async def test_subagent_receives_no_plan_mode_injection(self) -> None: + # Subagents share the session's plan_mode flag (for persistence/resume), + # but their YAML usually excludes EnterPlanMode/ExitPlanMode. Injecting the + # plan-mode workflow reminder would only invite hallucinated tool calls, so + # the provider must suppress it for subagents even while plan mode is active. + provider = PlanModeInjectionProvider() + soul = _make_soul_mock( + plan_mode=True, + plan_path=Path("/tmp/plan.md"), + is_subagent=True, + ) + + result = await provider.get_injections([], soul) + + assert result == [] + class TestPlanModeVerificationClause: """planning-1 backfill: lock the mandatory Verification-section requirement diff --git a/tests/ui_and_conv/test_shell_switch_slash.py b/tests/ui_and_conv/test_shell_switch_slash.py index f8a0fa11..aaa4eea7 100644 --- a/tests/ui_and_conv/test_shell_switch_slash.py +++ b/tests/ui_and_conv/test_shell_switch_slash.py @@ -1,6 +1,6 @@ -"""Tests for /web and /vis slash commands and their exception propagation. +"""Tests for /web and /reports slash commands and their exception propagation. -Ensures that typing /web or /vis in the interactive shell cleanly switches +Ensures that typing /web or /reports in the interactive shell cleanly switches to the corresponding server without hanging or corrupting terminal state. """ @@ -115,40 +115,40 @@ async def test_does_not_raise_switch_to_vis(self) -> None: # --------------------------------------------------------------------------- -# /vis — registration +# /reports — registration # --------------------------------------------------------------------------- -class TestVisCommandRegistration: - """Verify /vis is registered in the correct registry.""" +class TestReportsCommandRegistration: + """Verify /reports is registered in the correct registry.""" def test_registered_in_shell_registry(self) -> None: - cmd = shell_slash_registry.find_command("vis") + cmd = shell_slash_registry.find_command("reports") assert cmd is not None - assert cmd.name == "vis" + assert cmd.name == "reports" assert "Visualizer" in cmd.description def test_not_in_shell_mode_registry(self) -> None: - assert shell_mode_registry.find_command("vis") is None + assert shell_mode_registry.find_command("reports") is None def test_not_in_soul_registry(self) -> None: from pythinker_code.soul.slash import registry as soul_slash_registry - assert soul_slash_registry.find_command("vis") is None + assert soul_slash_registry.find_command("reports") is None # --------------------------------------------------------------------------- -# /vis — behaviour +# /reports — behaviour # --------------------------------------------------------------------------- -class TestVisCommandBehavior: - """Verify /vis raises SwitchToVis with the current session ID.""" +class TestReportsCommandBehavior: + """Verify /reports raises SwitchToVis with the current session ID.""" async def test_raises_switch_to_vis(self) -> None: shell = _mock_shell_with_soul("my-session-123") - cmd = shell_slash_registry.find_command("vis") + cmd = shell_slash_registry.find_command("reports") assert cmd is not None with pytest.raises(SwitchToVis) as exc_info: @@ -159,7 +159,7 @@ async def test_raises_switch_to_vis(self) -> None: async def test_carries_session_id(self) -> None: shell = _mock_shell_with_soul("abc-def") - cmd = shell_slash_registry.find_command("vis") + cmd = shell_slash_registry.find_command("reports") assert cmd is not None with pytest.raises(SwitchToVis) as exc_info: @@ -172,7 +172,7 @@ async def test_session_id_none_without_pythinker_soul(self) -> None: shell = Mock() shell.soul = Mock() - cmd = shell_slash_registry.find_command("vis") + cmd = shell_slash_registry.find_command("reports") assert cmd is not None with pytest.raises(SwitchToVis) as exc_info: @@ -181,10 +181,10 @@ async def test_session_id_none_without_pythinker_soul(self) -> None: assert exc_info.value.session_id is None async def test_does_not_raise_switch_to_web(self) -> None: - """/vis must raise SwitchToVis, not SwitchToWeb.""" + """/reports must raise SwitchToVis, not SwitchToWeb.""" shell = _mock_shell_with_soul() - cmd = shell_slash_registry.find_command("vis") + cmd = shell_slash_registry.find_command("reports") assert cmd is not None with pytest.raises(SwitchToVis): @@ -271,26 +271,26 @@ def thrower(*args: Any, _exc: Exception = exc, **kwargs: Any) -> None: # --------------------------------------------------------------------------- -# /web + /vis — coexistence +# /web + /reports — coexistence # --------------------------------------------------------------------------- -class TestWebAndVisCoexistence: - """Verify /web and /vis coexist without interference.""" +class TestWebAndReportsCoexistence: + """Verify /web and /reports coexist without interference.""" def test_both_registered(self) -> None: web_cmd = shell_slash_registry.find_command("web") - vis_cmd = shell_slash_registry.find_command("vis") + vis_cmd = shell_slash_registry.find_command("reports") assert web_cmd is not None assert vis_cmd is not None assert web_cmd.name != vis_cmd.name async def test_same_shell_different_exceptions(self) -> None: - """Given the same shell, /web raises SwitchToWeb and /vis raises SwitchToVis.""" + """Given the same shell, /web raises SwitchToWeb and /reports raises SwitchToVis.""" shell = _mock_shell_with_soul("shared-session") web_cmd = shell_slash_registry.find_command("web") - vis_cmd = shell_slash_registry.find_command("vis") + vis_cmd = shell_slash_registry.find_command("reports") assert web_cmd is not None assert vis_cmd is not None diff --git a/tests/ui_and_conv/test_shell_welcome_info.py b/tests/ui_and_conv/test_shell_welcome_info.py index 0cb1d529..0c79eda4 100644 --- a/tests/ui_and_conv/test_shell_welcome_info.py +++ b/tests/ui_and_conv/test_shell_welcome_info.py @@ -143,8 +143,9 @@ def test_welcome_banner_layout_width_matrix(monkeypatch): output = console.export_text() lines = [line.rstrip() for line in output.splitlines() if line.strip()] - max_panel_width = min(width, shell_module._WELCOME_MAX_WIDTH) - assert all(cell_width(line) <= max_panel_width for line in lines) + # The panel spans the full terminal width, like the prompt rules. + assert any(cell_width(line) == width for line in lines) + assert all(cell_width(line) <= width for line in lines) assert "Pythinker Code v9.9.9" in lines[0] assert "Welcome to Pythinker" in output assert "Directory" in output @@ -158,6 +159,98 @@ def test_welcome_banner_layout_width_matrix(monkeypatch): assert "▛" in output +def test_welcome_two_column_layout_when_wide(monkeypatch): + from pythinker_code.ui.shell import WelcomeInfoItem + + console = Console(record=True, width=120, color_system=None) + monkeypatch.setattr(shell_module, "console", console) + monkeypatch.setattr(shell_module, "get_version", lambda: "9.9.9") + monkeypatch.setattr(shell_module, "ascii_glyphs_enabled", lambda: False) + + items = [ + WelcomeInfoItem(name="Directory", value="/tmp/proj"), + WelcomeInfoItem(name="Tip", value="Type /help for commands."), + ] + shell_module._print_welcome_info("Pythinker Code", items) + + lines = console.export_text().splitlines() + # Welcome copy and the Tips column share the first content row, separated + # by the vertical divider (panel edges + divider = 3 pipes). + head_line = next(ln for ln in lines if "Welcome to Pythinker" in ln) + assert "Tips" in head_line + assert head_line.count("│") == 3 + # The robot mark renders in the left column. + assert any("▛" in ln for ln in lines) + # Facts sit inside the left column, so the divider crosses their rows too. + dir_line = next(ln for ln in lines if "Directory" in ln) + assert dir_line.count("│") == 3 + assert "/tmp/proj" in dir_line + + +def test_welcome_ascii_mode_emits_pure_ascii(monkeypatch): + from pythinker_code.ui.shell import WelcomeInfoItem + + console = Console(record=True, width=120, color_system=None) + monkeypatch.setattr(shell_module, "console", console) + monkeypatch.setattr(shell_module, "get_version", lambda: "9.9.9") + monkeypatch.setattr(shell_module, "ascii_glyphs_enabled", lambda: True) + + items = [ + WelcomeInfoItem( + name="Auto-save", + value="~/.pythinker/sessions/" + "a" * 120 + "/context.json", + ), + WelcomeInfoItem(name="Tip", value="No AGENTS.md found — run /init to generate one."), + ] + shell_module._print_welcome_info("Pythinker Code", items) + + output = console.export_text() + assert output.isascii(), [ch for ch in set(output) if not ch.isascii()] + assert "Welcome to Pythinker" in output + assert "Tips" in output + + +def test_welcome_chip_degrades_to_ascii(monkeypatch): + monkeypatch.setattr(shell_module, "consume_whats_new", lambda: None) + monkeypatch.setattr(shell_module, "welcome_update_target", lambda: "1.2.3") + monkeypatch.setattr(shell_module, "ascii_glyphs_enabled", lambda: True) + + chip = shell_module._welcome_banner_chip() + + assert chip is not None + assert chip.plain.isascii() + assert "Update available" in chip.plain + + +def test_logo_antenna_blinks_unless_motion_disabled(monkeypatch): + monkeypatch.setattr(shell_module, "motion_disabled", lambda: False) + spans = shell_module._logo_text().spans + assert any("blink" in str(span.style) for span in spans) + + monkeypatch.setattr(shell_module, "motion_disabled", lambda: True) + spans = shell_module._logo_text().spans + assert not any("blink" in str(span.style) for span in spans) + + +def test_welcome_tiny_width_does_not_crash(monkeypatch): + from pythinker_code.ui.shell import WelcomeInfoItem + from pythinker_code.ui.shell.components.render_utils import cell_width + + console = Console(record=True, width=30, color_system=None) + monkeypatch.setattr(shell_module, "console", console) + monkeypatch.setattr(shell_module, "get_version", lambda: "9.9.9") + + items = [ + WelcomeInfoItem(name="Directory", value="/very/long/path/that/never/ends/project"), + WelcomeInfoItem(name="Tip", value="Type /help for commands."), + ] + shell_module._print_welcome_info("Pythinker Code", items) + + lines = [ln.rstrip() for ln in console.export_text().splitlines() if ln.strip()] + assert lines + assert all(cell_width(ln) <= 30 for ln in lines) + + def test_welcome_auto_save_path_is_middle_truncated_not_wrapped(monkeypatch): from pythinker_code.ui.shell import WelcomeInfoItem diff --git a/tests/ui_and_conv/test_worklog_render.py b/tests/ui_and_conv/test_worklog_render.py index e6b8c1a7..fd28e0bb 100644 --- a/tests/ui_and_conv/test_worklog_render.py +++ b/tests/ui_and_conv/test_worklog_render.py @@ -164,6 +164,17 @@ def test_brief_display_block_renders_report_card_when_multiline(): assert "Line two" in output +def test_error_brief_renders_stderr_tail_as_plain_text(): + # Error briefs now carry a trailing stderr tail. Rendering it as Markdown + # would mangle shell output — backticks, '#', '*' are Markdown syntax — so + # error briefs must render verbatim. + brief = "Failed with exit code: 1\nValueError: bad `config` value" + output = _plain(render_display_blocks([BriefDisplayBlock(text=brief)], is_error=True)[0]) + + assert "`config`" in output # backticks preserved -> not parsed as Markdown + assert "Failed with exit code: 1" in output + + def test_consecutive_diff_blocks_for_same_file_render_one_card(): cards = render_display_blocks( [ diff --git a/tests/utils/test_result_builder.py b/tests/utils/test_result_builder.py index c5d2e6d6..dcba3520 100644 --- a/tests/utils/test_result_builder.py +++ b/tests/utils/test_result_builder.py @@ -157,6 +157,50 @@ def test_empty_write(): assert not builder.is_full +def test_tail_empty(): + """tail() on an empty buffer returns an empty string.""" + builder = ToolResultBuilder() + assert builder.tail() == "" + + +def test_tail_basic(): + """tail() returns the trailing lines, oldest-to-newest, no trailing newline.""" + builder = ToolResultBuilder() + builder.write("first line\nsecond line\nthird line\n") + assert builder.tail() == "first line\nsecond line\nthird line" + + +def test_tail_skips_blank_lines(): + """Blank/whitespace-only lines are skipped so the tail carries real context.""" + builder = ToolResultBuilder() + builder.write("real error\n\n \n") + assert builder.tail() == "real error" + + +def test_tail_respects_max_lines(): + """tail() returns at most max_lines lines, taken from the end.""" + builder = ToolResultBuilder() + builder.write("\n".join(f"line {i}" for i in range(10)) + "\n") + assert builder.tail(max_lines=3) == "line 7\nline 8\nline 9" + + +def test_tail_truncates_long_line(): + """Over-long lines are clipped to max_line_len and suffixed with an ellipsis.""" + builder = ToolResultBuilder() + builder.write("x" * 500 + "\n") + tail = builder.tail(max_line_len=100) + assert tail.endswith("...") + assert len(tail) == 103 + + +def test_tail_handles_multiple_writes(): + """tail() spans separate write() calls (e.g. interleaved stdout/stderr).""" + builder = ToolResultBuilder() + builder.write("stdout chunk\n") + builder.write("stderr: permission denied\n") + assert builder.tail(max_lines=2) == "stdout chunk\nstderr: permission denied" + + def test_spill_on_truncation_saves_full_output_and_hints(tmp_path): """tooldesc-2/ctxmgmt-1: truncated foreground output spills to disk with a recovery hint instead of being silently discarded.""" From 529a5beba53144c3c04106c25bae68d5b8407191 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Thu, 11 Jun 2026 00:32:24 -0400 Subject: [PATCH 4/9] feat(soul): dedupe repeated tool calls within and across steps Identical tool calls in one step now share the original task's result instead of re-executing. Across steps, repeats are detected via canonical JSON arguments (key order no longer defeats matching) and nudged with sparse system-reminders at consecutive streaks of 3, 5, and 8, reducing loop-thrash without hard-blocking legitimate retries. Per-step state is armed inside the step-retry wrapper so a retried step never awaits tasks cancelled by the failed attempt, and a D-Mail revert clears the dedup seed since the reverted history no longer contains those calls. Adds tool_call_dedup_detected telemetry plus a dup_type property on tool_call events. --- src/pythinker_code/soul/pythinkersoul.py | 21 ++ src/pythinker_code/soul/toolset.py | 221 +++++++++++++++++- tests/core/test_toolset.py | 280 ++++++++++++++++++++++- 3 files changed, 519 insertions(+), 3 deletions(-) diff --git a/src/pythinker_code/soul/pythinkersoul.py b/src/pythinker_code/soul/pythinkersoul.py index e79c3a1f..f4f07df2 100644 --- a/src/pythinker_code/soul/pythinkersoul.py +++ b/src/pythinker_code/soul/pythinkersoul.py @@ -420,6 +420,10 @@ def __init__( self._steer_queue: asyncio.Queue[str | list[ContentPart]] = asyncio.Queue() self._prompt_queue_lock = asyncio.Lock() + # Tool calls made in the previous step, fed to the toolset's dedup + # tracking at the start of each step (see PythinkerToolset.begin_step). + self._last_tool_calls: list[tuple[str, str]] = [] + self._current_turn_id: str = "" self._plan_mode: bool = self._runtime.session.state.plan_mode self._plan_session_id: str | None = self._runtime.session.state.plan_session_id # Pre-warm slug cache so the persisted slug survives process restarts @@ -1105,6 +1109,8 @@ async def _turn(self, user_message: Message) -> TurnOutcome: if missing_caps := check_message(user_message, self._runtime.llm.capabilities): raise LLMNotSupported(self._runtime.llm, list(missing_caps)) + self._current_turn_id = uuid.uuid4().hex + self._last_tool_calls = [] self._sleep_inhibitor.set_turn_running(True) try: bus = shared_event_bus() @@ -1451,6 +1457,9 @@ async def _agent_loop(self) -> TurnOutcome: if back_to_the_future is not None: await self._context.revert_to(back_to_the_future.checkpoint_id) + # The reverted history no longer contains the last step's calls, + # so they must not seed cross-step dedup for the next step. + self._last_tool_calls = [] await self._checkpoint() await self._context.append_message(back_to_the_future.messages) @@ -1529,6 +1538,14 @@ def _on_tool_result(tool_result: ToolResult) -> None: wire_send(tool_result) async def _run_step_once() -> StepResult: + # Reset per-step dedup state. Inside the retry wrapper on purpose: a + # retried step must not await tool tasks cancelled by the failed attempt. + if isinstance(self._agent.toolset, PythinkerToolset): + self._agent.toolset.begin_step( + self._last_tool_calls, + step_no=self._current_step_no, + turn_id=self._current_turn_id, + ) # run an LLM step (may be interrupted) from pythinker_code.telemetry import metrics as _m from pythinker_code.telemetry import otel as _otel @@ -1715,6 +1732,10 @@ async def _pythinker_core_step_with_retry() -> StepResult: raise logger.debug("Got tool results: {results}", results=results) + # Update dedup tracking for the next step + if isinstance(self._agent.toolset, PythinkerToolset): + self._last_tool_calls = self._agent.toolset.end_step() + # If a tool (EnterPlanMode/ExitPlanMode) changed plan mode during execution, # send a corrected StatusUpdate so the client sees the up-to-date state. if self._plan_mode != plan_mode_before_tools: diff --git a/src/pythinker_code/soul/toolset.py b/src/pythinker_code/soul/toolset.py index a0e27bd7..2b0b973c 100644 --- a/src/pythinker_code/soul/toolset.py +++ b/src/pythinker_code/soul/toolset.py @@ -3,6 +3,7 @@ import asyncio import contextlib import difflib +import hashlib import importlib import inspect import json @@ -13,7 +14,7 @@ from dataclasses import dataclass from datetime import timedelta from pathlib import Path -from typing import TYPE_CHECKING, Any, Literal, overload +from typing import TYPE_CHECKING, Any, Literal, cast, overload from pythinker_core.tooling import ( CallableTool, @@ -208,6 +209,7 @@ def _set_transport_log_file(transport: Any) -> None: type ToolType = CallableTool | CallableTool2[Any] +type ToolCallKey = tuple[str, str] if TYPE_CHECKING: @@ -216,6 +218,83 @@ def type_check(pythinker_toolset: PythinkerToolset): _: Toolset = pythinker_toolset +_REMINDER_TEXT_1 = ( + "\n\n\n" + "You are repeating the exact same tool call with identical parameters." + " Please carefully analyze the previous result. If the task is not yet complete," + " try a different method or parameters instead of repeating the same call." + "\n" +) + + +def _make_reminder_text_2(tool_name: str, repeat_count: int, canonical_args: str) -> str: + return ( + "\n\n\n" + "You have repeatedly called the same tool with identical parameters many times.\n" + "Repeated tool call detected:\n" + f"- tool: {tool_name}\n" + f"- repeated_times: {repeat_count}\n" + f"- arguments: {canonical_args}\n" + "The previous repeated calls did not make progress. Do not call this exact same tool " + "with the exact same arguments again.\n" + "Carefully inspect the latest tool result and choose a different next action, " + "different parameters, or finish the task if enough evidence has been gathered." + "\n" + ) + + +def _sort_json_value(value: object) -> object: + if isinstance(value, list): + return [_sort_json_value(item) for item in cast("list[object]", value)] + if isinstance(value, dict): + value_dict = cast("dict[str, object]", value) + return {key: _sort_json_value(value_dict[key]) for key in sorted(value_dict)} + return value + + +def _canonical_tool_arguments(arguments: Any) -> str: + try: + return json.dumps( + _sort_json_value(arguments), + ensure_ascii=False, + separators=(",", ":"), + ) + except (TypeError, ValueError): + return str(arguments) + + +def _canonical_tool_arguments_text(arguments: str) -> str: + try: + return _canonical_tool_arguments(json.loads(arguments, strict=False)) + except json.JSONDecodeError: + return arguments + + +def _normalize_call_key(tool_name: str, arguments: str) -> ToolCallKey: + return (tool_name, _canonical_tool_arguments_text(arguments)) + + +def _append_reminder_to_return_value( + return_value: Any, reminder_text: str = _REMINDER_TEXT_1 +) -> Any: + """Append dedup reminder text to a ToolReturnValue output.""" + if not isinstance(return_value, ToolReturnValue): + return return_value + + output = return_value.output + + if isinstance(output, str): + new_output: str | list[ContentPart] = output + reminder_text + else: + new_output = list(output) + if new_output and isinstance(new_output[-1], TextPart): + new_output[-1] = TextPart(text=new_output[-1].text + reminder_text) + else: + new_output.append(TextPart(text=reminder_text)) + + return return_value.model_copy(update={"output": new_output}) + + class PythinkerToolset: def __init__(self, runtime: Runtime | None = None) -> None: self._runtime = runtime @@ -226,6 +305,18 @@ def __init__(self, runtime: Runtime | None = None) -> None: self._deferred_mcp_load: tuple[list[MCPConfig], Runtime] | None = None self._hook_engine: HookEngine = HookEngine() + # Deduplication state + self._previous_step_calls: list[ToolCallKey] = [] + self._current_step_calls: list[ToolCallKey] = [] + self._current_step_tasks: dict[ToolCallKey, asyncio.Task[ToolResult]] = {} + self._seen_call_keys: set[ToolCallKey] = set() + self._consecutive_key: ToolCallKey | None = None + self._consecutive_count: int = 0 + self._step_closed: bool = False + self._dedup_triggered: bool = False + self._step_no: int = 0 + self._turn_id: str = "" + def set_hook_engine(self, engine: HookEngine) -> None: self._hook_engine = engine @@ -332,6 +423,64 @@ def _is_tool_visible(self, tool: ToolType) -> bool: return True + def begin_step( + self, + previous_calls: list[tuple[str, str]], + *, + step_no: int = 0, + turn_id: str = "", + ) -> None: + """Called before each step to set up deduplication state.""" + self._previous_step_calls = [ + _normalize_call_key(tool_name, arguments) for tool_name, arguments in previous_calls + ] + self._current_step_calls = [] + self._current_step_tasks = {} + self._step_closed = False + self._dedup_triggered = False + self._step_no = step_no + self._turn_id = turn_id + if not self._previous_step_calls: + self._seen_call_keys = set() + self._consecutive_key = None + self._consecutive_count = 0 + else: + self._seen_call_keys.update(self._previous_step_calls) + if self._consecutive_key is None and self._consecutive_count == 0: + self._advance_consecutive_streak(self._previous_step_calls) + + def end_step(self) -> list[tuple[str, str]]: + """Called after each step to capture the calls made in this step.""" + if not self._step_closed: + self._advance_consecutive_streak(self._current_step_calls) + self._seen_call_keys.update(self._current_step_calls) + self._step_closed = True + return list(self._current_step_calls) + + def _advance_consecutive_streak(self, calls: list[ToolCallKey]) -> None: + for call_key in calls: + if call_key == self._consecutive_key: + self._consecutive_count += 1 + else: + self._consecutive_key = call_key + self._consecutive_count = 1 + + def _projected_streak_for_call(self, call_index: int) -> int: + consecutive_key = self._consecutive_key + consecutive_count = self._consecutive_count + for call_key in self._current_step_calls[: call_index + 1]: + if call_key == consecutive_key: + consecutive_count += 1 + else: + consecutive_key = call_key + consecutive_count = 1 + return consecutive_count + + @property + def dedup_triggered(self) -> bool: + """Whether a cross-step duplicate was blocked in the current step.""" + return self._dedup_triggered + def handle(self, tool_call: ToolCall) -> HandleResult: token = current_tool_call.set(tool_call) try: @@ -361,6 +510,56 @@ def handle(self, tool_call: ToolCall) -> HandleResult: ) return ToolResult(tool_call_id=tool_call.id, return_value=ToolParseError(str(e))) + canonical_args = _canonical_tool_arguments(arguments) + call_key = (tool_call.function.name, canonical_args) + call_index = len(self._current_step_calls) + self._current_step_calls.append(call_key) + + # Same-step dedup: wait for the original task and copy its result. + if call_key in self._current_step_tasks: + from pythinker_code.telemetry import track + + track( + "tool_call_dedup_detected", + turn_id=self._turn_id, + step_no=self._step_no, + tool_name=tool_call.function.name, + dup_type="same_step", + args_hash=hashlib.sha256(canonical_args.encode("utf-8")).hexdigest()[:8], + ) + original_task = self._current_step_tasks[call_key] + + async def _await_dup() -> ToolResult: + original_result = await original_task + return ToolResult( + tool_call_id=tool_call.id, + return_value=original_result.return_value, + ) + + return asyncio.create_task(_await_dup()) + + is_cross_step_dup = call_key in self._seen_call_keys + reminder_text: str | None = None + if is_cross_step_dup: + from pythinker_code.telemetry import track + + track( + "tool_call_dedup_detected", + turn_id=self._turn_id, + step_no=self._step_no, + tool_name=tool_call.function.name, + dup_type="cross_step", + args_hash=hashlib.sha256(canonical_args.encode("utf-8")).hexdigest()[:8], + ) + self._dedup_triggered = True + repeat_count = self._projected_streak_for_call(call_index) + if repeat_count == 3: + reminder_text = _REMINDER_TEXT_1 + elif repeat_count in (5, 8): + reminder_text = _make_reminder_text_2( + tool_call.function.name, repeat_count, canonical_args + ) + async def _call(): started_ids_token = _current_tool_execution_started_ids.set(set[str]()) try: @@ -472,6 +671,7 @@ async def _call_with_lifecycle(): success=False, duration_ms=int(tool_elapsed * 1000), error_type=_error_type, + dup_type="cross_step" if is_cross_step_dup else "normal", ) return ToolResult( tool_call_id=tool_call.id, @@ -509,6 +709,7 @@ async def _call_with_lifecycle(): tool_name=tool_call.function.name, success=not isinstance(ret, ToolError), duration_ms=int(tool_elapsed * 1000), + dup_type="cross_step" if is_cross_step_dup else "normal", ) # --- PostToolUse (fire-and-forget) --- @@ -527,7 +728,23 @@ async def _call_with_lifecycle(): return ToolResult(tool_call_id=tool_call.id, return_value=ret) - return asyncio.create_task(_call()) + task = asyncio.create_task(_call()) + if reminder_text is not None: + + async def _wrap_with_reminder( + inner_task: asyncio.Task[ToolResult], + text: str, + ) -> ToolResult: + tr = await inner_task + return ToolResult( + tool_call_id=tr.tool_call_id, + return_value=_append_reminder_to_return_value(tr.return_value, text), + ) + + task = asyncio.create_task(_wrap_with_reminder(task, reminder_text)) + + self._current_step_tasks[call_key] = task + return task finally: current_tool_call.reset(token) diff --git a/tests/core/test_toolset.py b/tests/core/test_toolset.py index 6ad50392..d6393b14 100644 --- a/tests/core/test_toolset.py +++ b/tests/core/test_toolset.py @@ -1,4 +1,4 @@ -"""Tests for PythinkerToolset hide/unhide functionality.""" +"""Tests for PythinkerToolset hide/unhide and deduplication functionality.""" from __future__ import annotations @@ -265,3 +265,281 @@ def test_mcp_tool_does_not_overwrite_existing_builtin() -> None: assert ts.find("ToolA") is original # A warning must have been logged about the conflict. assert any("ToolA" in msg for msg in warnings) + + +# --- deduplication --- + + +async def test_same_step_dedup(): + """Duplicate tool calls within the same step should share the original result.""" + ts = _make_toolset() + ts.begin_step([]) + + args = json.dumps({"value": "x"}) + tool_call_1 = ToolCall( + id="tc-dedup-1", + function=ToolCall.FunctionBody( + name="ToolA", + arguments=args, + ), + ) + tool_call_2 = ToolCall( + id="tc-dedup-2", + function=ToolCall.FunctionBody( + name="ToolA", + arguments=args, + ), + ) + + result_1 = ts.handle(tool_call_1) + assert isinstance(result_1, asyncio.Task) + + result_2 = ts.handle(tool_call_2) + assert isinstance(result_2, asyncio.Task) + + # Both should eventually return the same output but with different tool_call_id + tr_1 = await result_1 + tr_2 = await result_2 + + assert tr_1.return_value.output == "a" + assert tr_2.return_value.output == "a" + assert tr_1.tool_call_id == "tc-dedup-1" + assert tr_2.tool_call_id == "tc-dedup-2" + + assert ts.end_step() == [("ToolA", '{"value":"x"}'), ("ToolA", '{"value":"x"}')] + + +async def test_same_step_dedup_canonicalizes_argument_key_order(): + """Equivalent JSON objects with different key order should share the original result.""" + ts = _make_toolset() + ts.begin_step([]) + + tool_call_1 = ToolCall( + id="tc-canonical-1", + function=ToolCall.FunctionBody( + name="ToolA", + arguments='{"a": 1, "b": 2}', + ), + ) + tool_call_2 = ToolCall( + id="tc-canonical-2", + function=ToolCall.FunctionBody( + name="ToolA", + arguments='{"b": 2, "a": 1}', + ), + ) + + result_1 = ts.handle(tool_call_1) + result_2 = ts.handle(tool_call_2) + assert isinstance(result_1, asyncio.Task) + assert isinstance(result_2, asyncio.Task) + + tr_1 = await result_1 + tr_2 = await result_2 + + assert tr_1.return_value.output == "a" + assert tr_2.return_value.output == "a" + assert ts.end_step() == [("ToolA", '{"a":1,"b":2}'), ("ToolA", '{"a":1,"b":2}')] + + +async def test_cross_step_duplicate_does_not_append_reminder_below_three_consecutive(): + """The second consecutive identical call is tracked but not reminded yet.""" + ts = _make_toolset() + args = json.dumps({"value": "x"}) + ts.begin_step([("ToolA", args)]) + + tool_call = ToolCall( + id="tc-dedup-reminder", + function=ToolCall.FunctionBody( + name="ToolA", + arguments=args, + ), + ) + + result = ts.handle(tool_call) + assert isinstance(result, asyncio.Task) + tr = await result + output = tr.return_value.output + assert isinstance(output, str) + assert output == "a" + assert ts.dedup_triggered is True + assert ts.end_step() == [("ToolA", '{"value":"x"}')] + + +async def test_cross_step_duplicate_appends_reminder_at_three_consecutive(): + """The first reminder is sparse and appears only at the third consecutive call.""" + ts = _make_toolset() + args = json.dumps({"value": "x"}) + previous_calls: list[tuple[str, str]] = [] + + for i in range(2): + ts.begin_step(previous_calls, step_no=i + 1) + result = ts.handle( + ToolCall( + id=f"tc-repeat-prior-{i}", + function=ToolCall.FunctionBody(name="ToolA", arguments=args), + ) + ) + assert isinstance(result, asyncio.Task) + tr = await result + assert "system-reminder" not in tr.return_value.output + previous_calls = ts.end_step() + + ts.begin_step(previous_calls, step_no=3) + result = ts.handle( + ToolCall( + id="tc-repeat-third", + function=ToolCall.FunctionBody(name="ToolA", arguments=args), + ) + ) + assert isinstance(result, asyncio.Task) + tr = await result + output = tr.return_value.output + assert isinstance(output, str) + assert "You are repeating the exact same tool call" in output + assert "repeated_times" not in output + + +async def test_cross_step_duplicate_uses_sparse_stronger_reminders(): + """The stronger reminder appears at the fifth repeat and includes canonical args.""" + ts = _make_toolset() + args = '{"b": 2, "a": 1}' + previous_calls: list[tuple[str, str]] = [] + last_output = "" + + for i in range(5): + ts.begin_step(previous_calls, step_no=i + 1) + result = ts.handle( + ToolCall( + id=f"tc-repeat-{i}", + function=ToolCall.FunctionBody(name="ToolA", arguments=args), + ) + ) + assert isinstance(result, asyncio.Task) + tr = await result + last_output = tr.return_value.output + previous_calls = ts.end_step() + + assert isinstance(last_output, str) + assert "You have repeatedly called the same tool" in last_output + assert "repeated_times: 5" in last_output + assert "tool: ToolA" in last_output + assert 'arguments: {"a":1,"b":2}' in last_output + + +async def test_non_duplicate_allowed(): + """A tool call with different arguments should be allowed even if the tool name matches.""" + ts = _make_toolset() + ts.begin_step([("ToolA", json.dumps({"value": "x"}))]) + + args = json.dumps({"value": "y"}) + tool_call = ToolCall( + id="tc-ok-1", + function=ToolCall.FunctionBody( + name="ToolA", + arguments=args, + ), + ) + + result = ts.handle(tool_call) + assert isinstance(result, asyncio.Task) + tr = await result + assert tr.return_value.output == "a" + assert ts.dedup_triggered is False + assert ts.end_step() == [("ToolA", '{"value":"y"}')] + + +def test_begin_end_step(): + """begin_step and end_step should correctly manage deduplication state.""" + ts = _make_toolset() + + ts.begin_step([("ToolA", "{}")]) + assert ts._previous_step_calls == [("ToolA", "{}")] + assert ts._current_step_calls == [] + assert ts._current_step_tasks == {} + assert ts.dedup_triggered is False + + ts._current_step_calls.append(("ToolB", "{}")) + assert ts.end_step() == [("ToolB", "{}")] + + # After end_step, internal lists are not cleared by end_step itself; + # the caller (PythinkerSoul) is expected to call begin_step again for the next step. + # But dedup_triggered should still reflect the last step's state. + assert ts.dedup_triggered is False + + +async def test_begin_step_resets_cancelled_tasks(): + """begin_step() must clear _current_step_tasks so a retry does not await a cancelled task.""" + ts = _make_toolset() + + ts.begin_step([], step_no=1, turn_id="t1") + args = json.dumps({"value": "x"}) + tc1 = ToolCall( + id="c1", + function=ToolCall.FunctionBody( + name="ToolA", + arguments=args, + ), + ) + result1 = ts.handle(tc1) + assert isinstance(result1, asyncio.Task) + result1.cancel() + + # Simulate retry: begin_step again for the same step + ts.begin_step([], step_no=1, turn_id="t1") + tc2 = ToolCall( + id="c2", + function=ToolCall.FunctionBody( + name="ToolA", + arguments=args, + ), + ) + result2 = ts.handle(tc2) + assert isinstance(result2, asyncio.Task) + assert result2 is not result1 + + # The new task should complete successfully (not raise CancelledError) + tr = await result2 + assert tr.return_value.output == "a" + + +async def test_cross_step_dedup_not_triggered_after_back_to_the_future(): + """When _last_tool_calls is emptied (back_to_the_future), the same call must not + be treated as a cross-step duplicate.""" + ts = _make_toolset() + + # Step 1: execute a tool + args = json.dumps({"value": "x"}) + ts.begin_step([], step_no=1, turn_id="t1") + tc1 = ToolCall( + id="c1", + function=ToolCall.FunctionBody( + name="ToolA", + arguments=args, + ), + ) + result1 = ts.handle(tc1) + assert isinstance(result1, asyncio.Task) + await result1 + last_calls = ts.end_step() + assert last_calls == [("ToolA", '{"value":"x"}')] + + # Simulate back_to_the_future: caller clears last_calls + last_calls = [] + + # Step 2: same call with empty last_calls should execute normally + ts.begin_step(last_calls, step_no=2, turn_id="t1") + tc2 = ToolCall( + id="c2", + function=ToolCall.FunctionBody( + name="ToolA", + arguments=args, + ), + ) + result2 = ts.handle(tc2) + assert isinstance(result2, asyncio.Task) + tr = await result2 + + # Should NOT have the cross-step reminder appended + assert tr.return_value.output == "a" + assert ts.dedup_triggered is False From de4ef5e39f2a22610c99f3fa28b3d76eba82a5f9 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Thu, 11 Jun 2026 00:32:33 -0400 Subject: [PATCH 5/9] chore: drop completed rename plan from todo, apply review nits The pythinker-cli -> pythinker-code rename plan is fully realized (root pyproject is pythinker-code, module is src/pythinker_code/, no pythinker_cli references remain), so retire it from tasks/todo.md. Review nits: annotate /reports with NoReturn and assert the injection counter stays untouched in the subagent plan-mode suppression test. --- src/pythinker_code/ui/shell/slash.py | 4 +- tasks/todo.md | 243 ++---------------- .../core/test_plan_mode_injection_provider.py | 1 + 3 files changed, 19 insertions(+), 229 deletions(-) diff --git a/src/pythinker_code/ui/shell/slash.py b/src/pythinker_code/ui/shell/slash.py index d1894678..4cdd1abd 100644 --- a/src/pythinker_code/ui/shell/slash.py +++ b/src/pythinker_code/ui/shell/slash.py @@ -3,7 +3,7 @@ import asyncio import re from collections.abc import Awaitable, Callable -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any, NoReturn, cast from prompt_toolkit.shortcuts.choice_input import ChoiceInput from rich.markup import escape @@ -1649,7 +1649,7 @@ def web(app: Shell, args: str): @registry.command -def reports(app: Shell, args: str): +def reports(app: Shell, args: str) -> NoReturn: """Open Pythinker session reports (Agent Tracing Visualizer) in browser""" from pythinker_code.telemetry import track diff --git a/tasks/todo.md b/tasks/todo.md index dd516995..41a51f6f 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,234 +1,23 @@ -# Plan: Full rename `pythinker-cli` → `pythinker-code` +# Tasks -**Status**: Planning — DO NOT execute yet. User to review and approve. -**Created**: 2026-05-07 -**Estimated effort**: 4–6 hours focused work, plus 1–2 hours of unanticipated breakage debugging. -**Risk**: HIGH. Touches 393 Python files (3,061 occurrences) plus binary build, web UI, telemetry, agents, examples, and 4 CI workflows. One missed reference can break the standalone binary build, the web UI, or runtime tool-loading. +## Active ---- +(none) -## Goal +## Recently completed -Make `pythinker-code` the canonical Python package and module name. After this rename: +### 2026-06-11 — Port upstream tool-call dedup (kimi-cli #2242 + #2372) -```bash -pip install pythinker-code # canonical install -pythinker --help # CLI command (unchanged) -python -c "import pythinker_code" # canonical import -``` +- `soul/toolset.py`: canonical args, same-step result sharing, cross-step sparse + reminders (streak 3/5/8), dedup telemetry. +- `soul/pythinkersoul.py`: per-turn reset, `begin_step` inside the step-retry + wrapper, `end_step` after tool results, D-Mail revert clears the dedup seed. +- `tests/core/test_toolset.py`: 9 upstream dedup tests ported (25 total green). +- Verified: full suite minus PTY e2e 4852 passed; `make check-pythinker-code` green. +- Skipped #2372 drive-bys (Kimi Code promo banner, /clear→/new alias change). -`pythinker-cli` (the PyPI name + Python module) is retired. Existing PyPI releases of `pythinker-cli==1.0.0` continue to work for anyone who installed them, but no new versions of `pythinker-cli` will ship. +### Dropped: `pythinker-cli` → `pythinker-code` rename plan (2026-05-07) ---- - -## Why this is risky - -Discovered during inventory: - -1. **Attribute naming, not just imports**: `joint_session.pythinker_cli_session` and `session.pythinker_cli_session` are property names used across `src/pythinker_cli/web/api/sessions.py` (10+ uses) and `src/pythinker_cli/web/runner/worker.py`. Renaming these is a structural code change, not a search-and-replace. - -2. **Dynamic tool imports** in agent YAMLs: `agents/default/coder.yaml` and others reference tool classes via dotted import path strings: `"pythinker_cli.tools.shell:Shell"`. Runtime errors if the import path is wrong, AND lots of these are in user-facing example agents. - -3. **PyInstaller binary build**: `pythinker.spec` line 4: `from pythinker_cli.utils.pyinstaller import datas, hiddenimports`, line 13: `["src/pythinker_cli/cli/__main__.py"]`. Standalone executables won't build if these aren't updated, and PyInstaller errors are notoriously cryptic. - -4. **Telemetry path regex**: `src/pythinker_cli/telemetry/sentry.py` filters stack frames with `r"^(.*?)(site-packages|pythinker_cli|src/pythinker_cli)/"`. After rename, error reports get noisier until this is fixed. - -5. **60+ references in `examples/`** including `pyproject.toml` dependencies — these are reference code users copy. They have to match the canonical name post-rename. - -6. **50+ docs files** (`docs/en/`, `tasks_ai/`, `AGENTS.md`, `CONTRIBUTING.md`, `README.md`) reference `pythinker-cli` and `pythinker_cli`. - -7. **Workflow files**: 4 `.github/workflows/release-pythinker-*.yml` files. The cli workflow file is referenced in PyPI's trusted publisher records — renaming the file means re-registering the publishers on PyPI dashboard. - -8. **PyPI dashboard state**: trusted publishers configured today reference `release-pythinker-cli.yml`. Either keep the workflow filename (less ideal — naming inconsistency) or rename and re-register on PyPI. - ---- - -## Scope summary - -| Item | Count | -|------|-------| -| Python files w/ `pythinker_cli` references | 393 | -| Total `pythinker_cli` occurrences in Python | 3,061 | -| Non-Python files w/ references | 50+ | -| Examples referencing the name | 60 | -| Workflow YAMLs to update | 4 | -| `pyproject.toml` files affected | 5 | -| Agent YAML files w/ dotted import paths | 5+ | -| Files touched in total | ~470 | - ---- - -## Architecture decision - -**Option A (chosen):** Make `packages/pythinker-code/` the new ROOT package. The current root `pyproject.toml` becomes a thin alias declaring `pythinker-cli` (kept for one release as a deprecation shim, then dropped in 1.1.0). - -**Why this layout:** -- The Python module rename `pythinker_cli` → `pythinker_code` is ONE tree move, not a directory swap -- Workspace tooling stays sane -- Existing GitHub Actions workflow filenames can stay (just updated content) -- We keep PyPI's trusted publisher registrations intact (workflow filenames stable) - -**Trade-off:** The root directory contains the alias instead of the canonical package, which is mildly weird structurally. But it's far less invasive than a full directory swap. - ---- - -## Phases - -### Phase 0 — Pre-flight (15 min) - -- [ ] **Confirm test suite passes on `main`** before any changes: - ```bash - uv sync --frozen --all-extras --all-packages - uv run pytest tests -x --co -q | head -30 # smoke: tests collect - ``` -- [ ] **Create branch** `rename/pythinker-code` from current `main` (db9545e or later): - ```bash - git switch -c rename/pythinker-code - ``` -- [ ] **Snapshot current state**: tag `pre-rename-snapshot` so we can `git diff` later: - ```bash - git tag pre-rename-snapshot - ``` -- [ ] **Document the old → new mapping** in this file (below) so we can grep-verify completeness. - -#### Name mappings reference - -| Old | New | -|-----|-----| -| `pythinker-cli` (PyPI name) | `pythinker-code` | -| `pythinker_cli` (Python module) | `pythinker_code` | -| `pythinker_cli_session` (attribute) | `pythinker_code_session` | -| `src/pythinker_cli/` | `src/pythinker_code/` | -| `release-pythinker-cli.yml` (workflow) | **KEEP** — see Phase 5 | -| `[tool.uv.workspace]` member `pythinker-cli` | `pythinker-code` | -| Telemetry/log service name `pythinker_cli` | `pythinker_code` | - -The CLI command `pythinker` stays. The `pythinker-cli` CLI alias also stays (we publish `pythinker-cli` as a thin alias package for one release). - ---- - -### Phase 1 — Module directory rename (30 min) - -Single atomic move with git so history is preserved: - -- [ ] `git mv src/pythinker_cli src/pythinker_code` -- [ ] Spot-check that `git log --follow src/pythinker_code/__main__.py` shows full history. -- [ ] DO NOT touch any file contents yet. Commit: - ```bash - git commit -m "chore: rename src/pythinker_cli/ -> src/pythinker_code/" - ``` - -At this point the tree is broken (393 files import a module that doesn't exist by that name). Phase 2 fixes it. - ---- - -### Phase 2 — Python import rewrite (60 min) - -This is the biggest mechanical change. Use `sed` for the bulk pass, then verify by hand. - -- [ ] **Bulk substitution** (preserving identifiers and strings): - ```bash - # All Python files - find src tests tests_ai tests_e2e packages sdks scripts examples -name "*.py" -type f \ - -exec sed -i 's/\bpythinker_cli\b/pythinker_code/g' {} + - ``` - -- [ ] **Verify no orphan `pythinker_cli` strings remain in *.py**: - - ```bash - grep -rn "pythinker_cli" --include="*.py" src/ packages/ sdks/ tests/ tests_e2e/ tests_ai/ scripts/ examples/ - ``` - - Expect: empty output (all references should now be `pythinker_code`). - -- [ ] **Verify `pythinker_code_session` attribute usage**: - Confirm that attribute references like `joint_session.pythinker_code_session` and `session.pythinker_code_session` are correctly updated in web API and worker files. - -- [ ] **Run the test collector** to confirm all imports resolve: - - ```bash - uv sync --frozen --all-extras --all-packages - uv run pytest tests --co -q 2>&1 | tail -20 - ``` - -- [ ] Commit. - ---- - -### Phase 3 — pyproject.toml + workspace surgery (45 min) - -- [ ] **Move root pyproject contents** to `packages/pythinker-code/pyproject.toml` -- [ ] **Decide root pyproject's fate** — Option C (recommended): keep as thin `pythinker-cli==1.1.0` alias for one release -- [ ] **Update `[tool.uv.workspace]`** members list -- [ ] **Adjust `[project.scripts]`** in `packages/pythinker-code/pyproject.toml` -- [ ] **Build all packages locally** -- [ ] Commit. - ---- - -### Phase 4 — Build/release infrastructure (30 min) - -- [ ] Update `pythinker.spec` (PyInstaller) -- [ ] Update `Makefile` targets -- [ ] Update `scripts/build_web.py` and `scripts/build_vis.py` -- [ ] Update `scripts/check_pythinker_dependency_versions.py` -- [ ] Local PyInstaller dry run -- [ ] Commit. - ---- - -### Phase 5 — Workflow files & PyPI publisher records (45 min) - -- [ ] Edit `.github/workflows/release-pythinker-cli.yml` (keep filename, update content) -- [ ] Update `scripts/check_version_tag.py` callsites -- [ ] Verify PyPI dashboard trusted publishers still valid -- [ ] Commit. - ---- - -### Phase 6 — Documentation, examples, agent YAMLs (60 min) - -- [ ] README.md, CONTRIBUTING.md, CHANGELOG.md -- [ ] `docs/en/**/*.md`, AGENTS.md, skills, tasks_ai -- [ ] `examples/**` (60 references — pyproject + READMEs + yamls) -- [ ] Agent YAMLs: tool import paths `"pythinker_cli.tools.*"` → `"pythinker_code.tools.*"` -- [ ] Commit. - ---- - -### Phase 7 — Verification (60 min) - -- [ ] Full test suite vs pre-rename-snapshot -- [ ] `uv sync` clean (rm .venv + uv.lock) -- [ ] pyright + ruff -- [ ] Smoke import + CLI run -- [ ] PyInstaller binary -- [ ] TestPyPI dry run - ---- - -### Phase 8 — Tag and release (30 min) - -- [ ] CHANGELOG 1.1.0 entry with migration notes -- [ ] Squash-merge to main and push tag -- [ ] Watch release workflow -- [ ] Verify on PyPI - ---- - -### Phase 9 — Post-release cleanup (deferred to 1.2.0) - -- [ ] Drop `pythinker-cli` alias package -- [ ] Rename workflow file + re-register PyPI publishers -- [ ] Remove `pythinker-cli` script entry -- [ ] Bump to 1.2.0 - ---- - -## Open questions — ANSWER BEFORE EXECUTION - -1. **Alias or cold-turkey?** Ship `pythinker-cli==1.1.0` as a one-shot deprecation alias (Option C), or drop at 1.1.0? -2. **Workflow filename**: keep `release-pythinker-cli.yml` for one release, or rename immediately? -3. **Module name**: confirm `pythinker_code` (vs. bare `pythinker`)? -4. **Migration callout** in README? -5. **Versioning**: breaking layout change at 1.1.0 or 2.0.0? +Obsolete — the rename is already fully realized: root `pyproject.toml` is +`name = "pythinker-code"`, the module is `src/pythinker_code/`, and zero +`pythinker_cli` references remain in source. diff --git a/tests/core/test_plan_mode_injection_provider.py b/tests/core/test_plan_mode_injection_provider.py index 585b867c..140278cf 100644 --- a/tests/core/test_plan_mode_injection_provider.py +++ b/tests/core/test_plan_mode_injection_provider.py @@ -151,6 +151,7 @@ async def test_subagent_receives_no_plan_mode_injection(self) -> None: result = await provider.get_injections([], soul) assert result == [] + assert provider._inject_count == 0 class TestPlanModeVerificationClause: From c294eb97aa2b619799077190495a58e95c4c27ed Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Thu, 11 Jun 2026 00:55:49 -0400 Subject: [PATCH 6/9] feat: editor bug fixes, Draft and auto save fixes. --- src/pythinker_code/auth/alibaba.py | 2 +- src/pythinker_code/soul/toolset.py | 26 +++++++++----------------- src/pythinker_code/ui/shell/slash.py | 2 +- 3 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/pythinker_code/auth/alibaba.py b/src/pythinker_code/auth/alibaba.py index a825290d..05fa5869 100644 --- a/src/pythinker_code/auth/alibaba.py +++ b/src/pythinker_code/auth/alibaba.py @@ -218,7 +218,7 @@ def _infer_capabilities(model_id: str) -> frozenset[ModelCapability] | None: if _VISION_RE.search(mid) or _QWEN_PLUS_RE.search(mid) or "kimi" in mid: caps.add("image_in") if _REASONING_RE.search(mid): - # DeepSeek/Moonshot expose a thinking dial; Qwen/GLM/MiniMax reason natively. + # DeepSeek/Kimi expose a thinking dial; Qwen/GLM/MiniMax reason natively. caps.add("thinking" if ("deepseek" in mid or "kimi" in mid) else "always_thinking") return frozenset(caps) or None diff --git a/src/pythinker_code/soul/toolset.py b/src/pythinker_code/soul/toolset.py index 2b0b973c..97e24942 100644 --- a/src/pythinker_code/soul/toolset.py +++ b/src/pythinker_code/soul/toolset.py @@ -274,9 +274,7 @@ def _normalize_call_key(tool_name: str, arguments: str) -> ToolCallKey: return (tool_name, _canonical_tool_arguments_text(arguments)) -def _append_reminder_to_return_value( - return_value: Any, reminder_text: str = _REMINDER_TEXT_1 -) -> Any: +def _append_reminder_to_return_value(return_value: Any, reminder_text: str) -> Any: """Append dedup reminder text to a ToolReturnValue output.""" if not isinstance(return_value, ToolReturnValue): return return_value @@ -726,23 +724,17 @@ async def _call_with_lifecycle(): ), ) - return ToolResult(tool_call_id=tool_call.id, return_value=ret) - - task = asyncio.create_task(_call()) - if reminder_text is not None: - - async def _wrap_with_reminder( - inner_task: asyncio.Task[ToolResult], - text: str, - ) -> ToolResult: - tr = await inner_task + # Append the dedup reminder inline (no-op on errors) so the + # returned task is the tool task itself: cancelling it cancels + # the tool, rather than orphaning it behind a wrapper task. + if reminder_text is not None: return ToolResult( - tool_call_id=tr.tool_call_id, - return_value=_append_reminder_to_return_value(tr.return_value, text), + tool_call_id=tool_call.id, + return_value=_append_reminder_to_return_value(ret, reminder_text), ) + return ToolResult(tool_call_id=tool_call.id, return_value=ret) - task = asyncio.create_task(_wrap_with_reminder(task, reminder_text)) - + task = asyncio.create_task(_call()) self._current_step_tasks[call_key] = task return task finally: diff --git a/src/pythinker_code/ui/shell/slash.py b/src/pythinker_code/ui/shell/slash.py index 4cdd1abd..77670f91 100644 --- a/src/pythinker_code/ui/shell/slash.py +++ b/src/pythinker_code/ui/shell/slash.py @@ -1653,7 +1653,7 @@ def reports(app: Shell, args: str) -> NoReturn: """Open Pythinker session reports (Agent Tracing Visualizer) in browser""" from pythinker_code.telemetry import track - track("vis_opened") + track("reports_opened") soul = ensure_pythinker_soul(app) session_id = soul.runtime.session.id if soul else None raise SwitchToVis(session_id=session_id) From f951b26dcca3ddf10d62fc58131cc089327e0550 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Thu, 11 Jun 2026 00:59:25 -0400 Subject: [PATCH 7/9] refactor: simplify dedup reminder path and tidy review nits soul/toolset: append the cross-step dedup reminder inline at the tool task's return instead of behind a second wrapper task. Behaviour is unchanged (toolset tests pass) but the returned task is now the tool task itself, with one fewer task and closure per reminder. Drop the now-unused default on _append_reminder_to_return_value and annotate begin_step/end_step with the existing ToolCallKey alias. tools/utils: tail() docstring no longer over-specifies the buffer as stderr (it holds mixed command output); rename stripped -> rstripped to reflect that it holds an rstrip() result. --- src/pythinker_code/soul/toolset.py | 4 ++-- src/pythinker_code/tools/utils.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/pythinker_code/soul/toolset.py b/src/pythinker_code/soul/toolset.py index 97e24942..2837c176 100644 --- a/src/pythinker_code/soul/toolset.py +++ b/src/pythinker_code/soul/toolset.py @@ -423,7 +423,7 @@ def _is_tool_visible(self, tool: ToolType) -> bool: def begin_step( self, - previous_calls: list[tuple[str, str]], + previous_calls: list[ToolCallKey], *, step_no: int = 0, turn_id: str = "", @@ -447,7 +447,7 @@ def begin_step( if self._consecutive_key is None and self._consecutive_count == 0: self._advance_consecutive_streak(self._previous_step_calls) - def end_step(self) -> list[tuple[str, str]]: + def end_step(self) -> list[ToolCallKey]: """Called after each step to capture the calls made in this step.""" if not self._step_closed: self._advance_consecutive_streak(self._current_step_calls) diff --git a/src/pythinker_code/tools/utils.py b/src/pythinker_code/tools/utils.py index 70fd7eec..6f7c7a3b 100644 --- a/src/pythinker_code/tools/utils.py +++ b/src/pythinker_code/tools/utils.py @@ -216,17 +216,17 @@ def write(self, text: str) -> int: def tail(self, max_lines: int = 5, max_line_len: int = 200) -> str: """Return the last non-empty lines from the buffer, joined with newlines. - Useful for surfacing actionable error context (stderr) in tool result briefs. + Useful for surfacing actionable error context in tool result briefs. """ collected: list[str] = [] for chunk in reversed(self._buffer): for line in reversed(chunk.splitlines()): - stripped = line.rstrip() - if not stripped.strip(): + rstripped = line.rstrip() + if not rstripped.strip(): continue - if len(stripped) > max_line_len: - stripped = stripped[:max_line_len] + "..." - collected.append(stripped) + if len(rstripped) > max_line_len: + rstripped = rstripped[:max_line_len] + "..." + collected.append(rstripped) if len(collected) >= max_lines: break if len(collected) >= max_lines: From 0f2dd93c0fe640a8a37c2d912c32d6826aba6b50 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Thu, 11 Jun 2026 04:39:18 -0400 Subject: [PATCH 8/9] fix: address CodeRabbit review on dedup reminder and test Bound the canonical arguments echoed in the strong dedup reminder to a 256-char preview so large-payload tools (WriteFile, MultiEdit) don't re-inject their whole body into context on every repeat; exact identity is still carried by the args_hash dedup telemetry. Rewrite test_begin_end_step to assert observable behaviour (handle()/end_step()/dedup_triggered) instead of poking private _current_step_* internals. --- src/pythinker_code/soul/toolset.py | 12 +++++++++++- tests/core/test_toolset.py | 15 +++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/pythinker_code/soul/toolset.py b/src/pythinker_code/soul/toolset.py index 2837c176..5e27483d 100644 --- a/src/pythinker_code/soul/toolset.py +++ b/src/pythinker_code/soul/toolset.py @@ -228,13 +228,23 @@ def type_check(pythinker_toolset: PythinkerToolset): def _make_reminder_text_2(tool_name: str, repeat_count: int, canonical_args: str) -> str: + # Echo only a bounded preview of the arguments: large-payload tools + # (WriteFile, MultiEdit) would otherwise re-inject the whole body into + # context on every repeat — defeating the reminder by inflating tokens. + # Exact identity is preserved by the args_hash in the dedup telemetry. + args_limit = 256 + if len(canonical_args) > args_limit: + dropped = len(canonical_args) - args_limit + args_preview = f"{canonical_args[:args_limit]}... [truncated {dropped} chars]" + else: + args_preview = canonical_args return ( "\n\n\n" "You have repeatedly called the same tool with identical parameters many times.\n" "Repeated tool call detected:\n" f"- tool: {tool_name}\n" f"- repeated_times: {repeat_count}\n" - f"- arguments: {canonical_args}\n" + f"- arguments: {args_preview}\n" "The previous repeated calls did not make progress. Do not call this exact same tool " "with the exact same arguments again.\n" "Carefully inspect the latest tool result and choose a different next action, " diff --git a/tests/core/test_toolset.py b/tests/core/test_toolset.py index d6393b14..a20e3cb9 100644 --- a/tests/core/test_toolset.py +++ b/tests/core/test_toolset.py @@ -449,22 +449,17 @@ async def test_non_duplicate_allowed(): assert ts.end_step() == [("ToolA", '{"value":"y"}')] -def test_begin_end_step(): - """begin_step and end_step should correctly manage deduplication state.""" +async def test_begin_end_step(): + """begin_step seeds the prior step's calls; end_step captures this step's.""" ts = _make_toolset() ts.begin_step([("ToolA", "{}")]) - assert ts._previous_step_calls == [("ToolA", "{}")] - assert ts._current_step_calls == [] - assert ts._current_step_tasks == {} assert ts.dedup_triggered is False - ts._current_step_calls.append(("ToolB", "{}")) + # A fresh (non-duplicate) call this step is captured by end_step() and does + # not trip cross-step dedup, since only ToolA was seen previously. + await ts.handle(ToolCall(id="b1", function=ToolCall.FunctionBody(name="ToolB", arguments="{}"))) assert ts.end_step() == [("ToolB", "{}")] - - # After end_step, internal lists are not cleared by end_step itself; - # the caller (PythinkerSoul) is expected to call begin_step again for the next step. - # But dedup_triggered should still reflect the last step's state. assert ts.dedup_triggered is False From 82dc122937d7e396fe12b9348d918504a63c26fe Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Thu, 11 Jun 2026 04:48:16 -0400 Subject: [PATCH 9/9] fix(test): narrow handle() result to Task before await (pyright) handle() returns Task[ToolResult] | ToolResult; assert isinstance Task before awaiting, matching the other dedup tests, so pyright's check job passes. --- tests/core/test_toolset.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/core/test_toolset.py b/tests/core/test_toolset.py index a20e3cb9..882bf15d 100644 --- a/tests/core/test_toolset.py +++ b/tests/core/test_toolset.py @@ -458,7 +458,11 @@ async def test_begin_end_step(): # A fresh (non-duplicate) call this step is captured by end_step() and does # not trip cross-step dedup, since only ToolA was seen previously. - await ts.handle(ToolCall(id="b1", function=ToolCall.FunctionBody(name="ToolB", arguments="{}"))) + result = ts.handle( + ToolCall(id="b1", function=ToolCall.FunctionBody(name="ToolB", arguments="{}")) + ) + assert isinstance(result, asyncio.Task) + await result assert ts.end_step() == [("ToolB", "{}")] assert ts.dedup_triggered is False