Skip to content

Commit 2904de0

Browse files
elkaixcoderabbitai[bot]CodeRabbit
authored
feat: tool-call dedup plus shell/terminal hardening and editor fixes (#116)
* chore: clean completed tasks from todo.md * fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * 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. * 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. * 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. * feat: editor bug fixes, Draft and auto save fixes. * 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. * 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. * 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. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
1 parent d016f8e commit 2904de0

40 files changed

Lines changed: 1048 additions & 1063 deletions

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ GitHub Releases page; `0.8.0` is the new starting line.
1515

1616
## Unreleased
1717

18+
- **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.
19+
- **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.
20+
- **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.
1821
- **New `/goal` command: goal-driven execution ported from Codex CLI.** `/goal <objective>` sets a persistent thread goal the agent pursues across turns until it is verifiably complete. The objective is stored in session state (survives restarts and context compaction), kicks off work immediately with a success-criteria derivation prompt, and is re-injected on later turns as a continuation reminder carrying Codex's fidelity rules (no scope-shrinking, no easier-to-test substitutes) and evidence-based completion audit — the agent may only claim completion after proving every requirement against current state, and the user confirms with `/goal clear`. Subcommands: `view`, `pause`, `resume`, `clear`. Objectives are injected as untrusted data (`<objective>` framing), never as higher-priority instructions.
1922
- **New `/best-practices` command (alias `/bp`).** Injects opt-in engineering best-practice guidance distilled from the Codex CLI system prompts — code-change discipline, dirty-worktree safety (never revert changes you didn't make), specific-to-broad testing strategy, todo hygiene, progress-update cadence, debugging methodology, and final-answer style — into the session context without consuming a turn. `/best-practices <section>` injects a single section.
2023
- **SetTodoList nudges the single-`in_progress` discipline.** Todo lists with more than one `in_progress` item now get a corrective notice (ported from Codex's plan-tool contract, softened because parallel-subagent fan-out legitimately tracks one `in_progress` sub-todo per running child), and the system prompt gains matching status-discipline guidance: no single-step lists, no `pending``done` jumps, no batch-completing after the fact.

docs/en/reference/pythinker-vis.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ The server automatically opens a browser after startup. The default address is `
1818

1919
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.
2020

21-
You can also type `/vis` in the interactive shell to switch directly from the current session to the Visualizer.
21+
You can also type `/reports` in the interactive shell to switch directly from the current session to the Visualizer.
2222

2323
## Command-line options
2424

docs/en/reference/slash-commands.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,9 +346,9 @@ Auto mode skips all approval confirmations and removes the clarifying-question s
346346

347347
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.
348348

349-
### `/vis`
349+
### `/reports`
350350

351-
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.
351+
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.
352352

353353
## Command completion
354354

src/pythinker_code/acp/tools.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,20 +149,22 @@ async def __call__(self, params: ShellParams) -> ToolReturnValue:
149149
else ""
150150
)
151151

152+
tail = builder.tail()
153+
tail_suffix = f"\n{tail}" if tail else ""
152154
if timed_out:
153155
return builder.error(
154156
f"Command killed by timeout ({timeout_label}){truncated_note}",
155-
brief=f"Killed by timeout ({timeout_label})",
157+
brief=f"Killed by timeout ({timeout_label}){tail_suffix}",
156158
)
157159
if exit_signal:
158160
return builder.error(
159161
f"Command terminated by signal: {exit_signal}.{truncated_note}",
160-
brief=f"Signal: {exit_signal}",
162+
brief=f"Signal: {exit_signal}{tail_suffix}",
161163
)
162164
if exit_code not in (None, 0):
163165
return builder.error(
164166
f"Command failed with exit code: {exit_code}.{truncated_note}",
165-
brief=f"Failed with exit code: {exit_code}",
167+
brief=f"Failed with exit code: {exit_code}{tail_suffix}",
166168
)
167169
return builder.ok(f"Command executed successfully.{truncated_note}")
168170
finally:

src/pythinker_code/app.py

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,6 @@ async def run_shell(
789789
branch_name = _safe_git_branch(work_dir)
790790
if branch_name:
791791
welcome_info.append(WelcomeInfoItem(name="Branch", value=branch_name))
792-
welcome_info.append(WelcomeInfoItem(name="Session", value=self._runtime.session.id))
793792
if notice := _resumed_unsupervised_notice(
794793
resumed=self._runtime.resumed,
795794
yolo=self._runtime.approval.is_yolo(),
@@ -798,14 +797,6 @@ async def run_shell(
798797
welcome_info.append(
799798
WelcomeInfoItem(name="Mode", value=notice, level=WelcomeInfoItem.Level.WARN)
800799
)
801-
try:
802-
auto_save_path = str(
803-
shorten_home(HostPath.unsafe_from_local_path(self._runtime.session.context_file))
804-
)
805-
except Exception:
806-
auto_save_path = ""
807-
if auto_save_path:
808-
welcome_info.append(WelcomeInfoItem(name="Auto-save", value=auto_save_path))
809800
if base_url := self._env_overrides.get("PYTHINKER_BASE_URL"):
810801
welcome_info.append(
811802
WelcomeInfoItem(
@@ -861,6 +852,18 @@ async def run_shell(
861852
level=WelcomeInfoItem.Level.WARN,
862853
)
863854
)
855+
# Session persistence details come last — workspace and model identity
856+
# read first, storage internals stay at the bottom of the facts block.
857+
welcome_info.append(WelcomeInfoItem(name="Session", value=self._runtime.session.id))
858+
try:
859+
auto_save_path = str(
860+
shorten_home(HostPath.unsafe_from_local_path(self._runtime.session.context_file))
861+
)
862+
except Exception:
863+
logger.debug("Failed to compute auto-save display path", exc_info=True)
864+
auto_save_path = ""
865+
if auto_save_path:
866+
welcome_info.append(WelcomeInfoItem(name="Auto-save", value=auto_save_path))
864867
welcome_info.append(
865868
WelcomeInfoItem(
866869
name="Tip",

src/pythinker_code/auth/openai/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ def _parse_chatgpt_models_payload(payload: object) -> list[ModelInfo]:
175175
raw_models = payload_object.get("models")
176176
if not isinstance(raw_models, list):
177177
# Keep a small compatibility path in case OpenAI ever aligns this with
178-
# the public /v1/models shape. ChatGPT Codex currently returns
178+
# the public /v1/models shape. The ChatGPT endpoint currently returns
179179
# {"models": [{"slug": ...}]}.
180180
raw_models = payload_object.get("data")
181181
if not isinstance(raw_models, list):

src/pythinker_code/auth/openai/oauth_client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ def _token_from_openai_response(payload: dict[str, Any]) -> OAuthToken:
184184
# it lives inside the OAuth JWT claims under
185185
# `https://api.openai.com/auth.chatgpt_account_id`. Hoist it onto the
186186
# response so OAuthToken.from_response() picks it up. Without this the
187-
# ChatGPT usage adapter, model catalog endpoint, and Codex request headers
187+
# ChatGPT usage adapter, model catalog endpoint, and request headers
188188
# cannot scope requests to the active Plus/Pro account.
189189
if "account_id" not in normalized:
190190
jwt_token = payload.get("id_token") or payload.get("access_token")

src/pythinker_code/auth/platforms.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ def _select_retry_api_keys(
176176

177177

178178
def _openai_fallback_models(platform_id: str) -> list[ModelInfo] | None:
179-
# ChatGPT Codex model availability is subscription/account-specific. Do not
179+
# ChatGPT model availability is subscription/account-specific. Do not
180180
# replace the user's live catalog with a static fallback; stale fallback
181181
# slugs surface as 400 "model is not supported with a ChatGPT account".
182182
if platform_id == OPENAI_CHATGPT_PLATFORM_ID:

src/pythinker_code/cli/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,7 @@ async def _run(session_id: str | None, prefill_text: str | None = None) -> tuple
911911
scratchpad_status = await ensure_git_excluded(work_dir)
912912

913913
# Sweep accumulated state on startup (best-effort, non-blocking).
914-
# Mirrors Claude Code's cleanupPeriodDays=30 model.
914+
# Default 30-day retention sweep.
915915
_retention = config.session_retention_days if isinstance(config, Config) else 30
916916
await asyncio.to_thread(sweep_old_sessions, _retention)
917917
await asyncio.to_thread(sweep_old_plans, _retention)

src/pythinker_code/hooks/runner.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ async def run_hook(
110110

111111

112112
def _extract_additional_context(parsed: dict[str, Any], hook_output: dict[str, Any]) -> str:
113-
"""Extract Claude-Code-style additionalContext from JSON hook output."""
113+
"""Extract ``additionalContext`` from JSON hook output."""
114114
candidates = (hook_output.get("additionalContext"), parsed.get("additionalContext"))
115115
for value in candidates:
116116
if isinstance(value, str) and value.strip():

0 commit comments

Comments
 (0)