Skip to content

fix(lint): migrate str+Enum to StrEnum; use PEP 695 type params - #11

Merged
elkaix merged 2 commits into
mainfrom
fix/ruff-up042-up047-str-enum
May 28, 2026
Merged

fix(lint): migrate str+Enum to StrEnum; use PEP 695 type params#11
elkaix merged 2 commits into
mainfrom
fix/ruff-up042-up047-str-enum

Conversation

@elkaix

@elkaix elkaix commented May 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Replaces class Foo(str, Enum) with class Foo(StrEnum) across 4 files in pythinker-review (9 enums total) to fix UP042
  • Replaces TypeVar("T") with PEP 695 type-parameter syntax in the ralph-loop test to fix UP047
  • All targets are Python 3.12+, so both StrEnum (3.11+) and PEP 695 (3.12+) are fully supported

Motivation

Unblocks the ruff 0.15 Dependabot bump (PR #4). Ruff 0.15 enabled UP042 and UP047 by default, which flagged 10 violations in existing code.

Test plan

  • CI check job passes (ruff UP042/UP047 clean)
  • All existing tests pass (no behavioral change — StrEnum is a drop-in for str+Enum when values are explicit strings)

Summary by CodeRabbit

  • Refactor
    • Updated internal enum implementations to use modern Python patterns for improved code consistency.
    • Modernized generic type annotations to align with current Python language standards.

Review Change Stack

…2, UP047)

Replace class Foo(str, Enum) with class Foo(StrEnum) across pythinker-review
(9 enums in cli/_shared, cli/review, engine/diff_source, store/models).
Replace TypeVar T with PEP 695 type-parameter syntax in the ralph-loop test.

All targets are Python 3.12+, so both StrEnum and PEP 695 are fully supported.
Unblocks the ruff 0.15 Dependabot bump (PR #4).
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8cd8efdb-a1bb-492e-aea7-97ee1549d9e2

📥 Commits

Reviewing files that changed from the base of the PR and between 65daa6c and 9918a13.

📒 Files selected for processing (5)
  • packages/pythinker-review/src/pythinker_review/cli/_shared.py
  • packages/pythinker-review/src/pythinker_review/cli/review.py
  • packages/pythinker-review/src/pythinker_review/engine/diff_source.py
  • packages/pythinker-review/src/pythinker_review/store/models.py
  • tests/core/test_pythinkersoul_ralph_loop.py

📝 Walkthrough

Walkthrough

This PR modernizes Python type handling across the codebase by systematically replacing (str, Enum) patterns with StrEnum (Python 3.11+) in CLI, engine, and model modules, and updates test generics to use PEP 695 syntax instead of TypeVar.

Changes

Type System Modernization

Layer / File(s) Summary
CLI enum migration to StrEnum
packages/pythinker-review/src/pythinker_review/cli/_shared.py, packages/pythinker-review/src/pythinker_review/cli/review.py
OutputFormat, FailOn, ReviewMode, ArtifactFormat, DiffSide, and SimilarIssuesBackend switched from inheriting (str, Enum) to StrEnum; import statement updated to use StrEnum instead of Enum.
Engine and model enum migration to StrEnum
packages/pythinker-review/src/pythinker_review/engine/diff_source.py, packages/pythinker-review/src/pythinker_review/store/models.py
DiffMode, Severity, and Category enums migrated from (str, Enum) to StrEnum; all member values and validation logic preserved.
Test generic syntax modernization
tests/core/test_pythinkersoul_ralph_loop.py
expect_snapshot function signature converted to PEP 695 generic parameter syntax expect_snapshot[T]; TypeVar import removed; RALPH_IMAGE_URL and RALPH_IMAGE_USER_INPUT constants repositioned earlier in module.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 70.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title follows conventional commits format (fix type, lint scope) and accurately describes the main changes: StrEnum migration and PEP 695 syntax adoption.
Description check ✅ Passed Description includes summary, motivation, and test plan with clear context. Related issue link and checklist items are not explicitly addressed but the core informational content is complete.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ruff-up042-up047-str-enum

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@elkaix
elkaix merged commit 45e38a5 into main May 28, 2026
17 checks passed
elkaix added a commit that referenced this pull request Jun 14, 2026
…eme parity, agent preview, MCP type filter) (#143)

* docs(tasks): add verified reference-adoption catalog

25-agent gap-analysis scout of the blackbox agent-harness reference vs current
pythinker. Of ~75 candidate patterns only 13 are actionable (1 adopt-now + 12
adapt); ~44 are already-have (pythinker is frequently more robust), the rest
stub-only/rewrite-defer/anti-pattern. Items are framed generically (technique,
not the reference's proprietary text) and fit the existing soul/ loop (no swap).
Planned in 4 waves: low-risk hardening, permission safety, loop/hook quality,
heavier/cross-package.

* feat(cli): add read-only `system-prompt` command to inspect the assembled prompt

Renders an agent's fully-assembled system prompt (work dir, OS/shell, merged
AGENTS.md, discovered skills) without creating a session, authenticating, or
loading MCP. Adds `render_agent_system_prompt` + `build_builtin_system_prompt_args`
in soul/agent.py (filesystem/environment only; mirrors the arg assembly in
Runtime.create for the read-only inspection path) and a lazy `system-prompt`
subcommand.

Wave 1 of the reference-adoption arc (adopt-now: dump-system-prompt-entrypoint).

* fix(shell): interpolate timeout caps into the tool description from the enforced constants

bash.md/powershell.md hardcoded the foreground (300s) and background (86400s)
timeout literals, which could silently drift from the MAX_FOREGROUND_TIMEOUT /
MAX_BACKGROUND_TIMEOUT constants that Params actually enforces. Replace the
literals with ${MAX_FOREGROUND_TIMEOUT}/${MAX_BACKGROUND_TIMEOUT} placeholders
rendered from the same constants, and add drift-guard tests (sentinel-render the
markdown; assert the live description states the enforced caps, never a raw
placeholder). Rendered output is identical for the current values.

Wave 1 of the reference-adoption arc (adapt: tool limits interpolated from enforced constants).

* feat(memory): warn that recalled notes are a point-in-time snapshot

The recall block already framed memory as past context that is not an
instruction, but did not warn that a recalled fact may be stale — a file, flag,
path, or decision it names may have changed or been removed. Add a freshness
caveat instructing the model to verify against current code before relying on
any recalled fact (failure-truthfulness C13: stale data must not read as
authoritative). Single injection site, so the caveat is inlined (no helper).

Wave 1 of the reference-adoption arc (adapt: per-memory freshness disclaimer).

* feat(toolset): bound parallel-safe tool fan-out with a concurrency cap

Parallel-safe tools previously overlapped without bound: a turn emitting many
parallel-safe calls (e.g. dozens of FetchURL) opened that many sockets/file
handles at once. Add an asyncio.Semaphore(10) to _ReadWriteGate acquired BEFORE
the writer-lock/counter bump so the cap throttles readers without affecting
writer draining (a reader queued on the cap has not incremented _active_readers,
so it never holds _readers_drained open, and writers never touch the semaphore).
Tests cover the cap and the no-deadlock writer-draining invariant.

Wave 1 of the reference-adoption arc (adapt: bounded parallel fan-out cap).

* feat(loop): enforce an optional per-session USD spend ceiling

Cost was accumulated and displayed but never enforced. Add
loop_control.max_session_cost_usd (off by default): when the session's accrued
estimated cost reaches the ceiling, the turn stops with a new budget_exhausted
outcome and a handoff message before starting another paid step, instead of
running to max_steps_per_turn. Goal auto-continuations already halt on any
non-no_tool_calls outcome; agent flows now halt on budget_exhausted too. Cost is
0 for unpriced models, so the ceiling fails open (never blocks un-estimable
spend). Decision/message logic are pure, boundary-tested helpers.

Wave 1 of the reference-adoption arc (adapt: per-session USD spend ceiling).

* feat(approval): always re-confirm edits to sensitive host files

Add an always-re-confirm deny-set for dangerous host paths so an auto-approve
tier can never silently approve them. New is_dangerous_host_path() flags shell
startup files, .git internals/hooks, .ssh, .vscode, and git credentials; a new
FileActions.EDIT_DANGEROUS is classified BEFORE the workspace branch (so a
.git/hooks write inside the workspace is no longer a plain EDIT). The approval
gate treats EDIT_DANGEROUS exactly like EDIT_CONFIG via a shared
_is_always_confirm_edit predicate: re-confirms even under yolo/auto, never
session-approved, denied with feedback in unattended runs. Security-reviewed
(SAFE TO MERGE): full parity with the config-edit gate, no fail-open bypass,
strict superset of prior protection.

Known parity limitation (pre-existing, shared with EDIT_CONFIG, deferred): a
.git/.ssh/.vscode directory that is itself a symlink evades name-matching after
realpath; Shell-tool writes bypass classify_edit_action entirely.

Wave 2 of the reference-adoption arc (adapt: dangerous-dotfile deny-set).

* feat(approval): add accept-edits auto-approve tier for reversible in-workspace edits

New /accept-edits toggle auto-approves only plain FileActions.EDIT (reversible,
in-workspace, non-destructive ordinary file edits) while shell, destructive,
outside-workspace, config-surface, and dangerous host edits still prompt. Adds a
session-local accept_edits flag (not persisted), _accept_edits_covers() gated by
safe_mode (like auto, unlike yolo), a request() auto-approve branch placed after
deliberation/unattended-deny, and the /accept-edits slash command. The enable
message is safe_mode-truthful (reports edits will still prompt under safe mode).

Pairs with the Wave 2 deny-set: dangerous paths classify as EDIT_DANGEROUS
(checked first), so they are never covered by EDIT auto-approval.
Security-reviewed (SAFE TO MERGE): over-approval bounded to EDIT only; EDIT is
produced by exactly one path (classify_edit_action); no persistence leak.

Wave 2 of the reference-adoption arc (adapt: accept-edits mode tier).

* feat(loop): add terminal-quality predicate (observable degenerate-stop signal)

Print mode exits 0 on any non-exception completion, including a stuck/empty
terminal that delivered no usable answer. Add a TurnOutcome.produced_answer
derived property: True only when the turn ended with a substantive assistant
answer (no_tool_calls stop with non-empty text); a forced handoff (stuck /
budget_exhausted), a rejected tool call, or an empty final message is False.
Surfaced on the existing turn span (turn.produced_answer) and turn.end event so
the degenerate-completion rate is measurable. Observational-first: exit-code
mapping is deliberately deferred until the rate is measured, to avoid
false-positives on tool-only-then-stop turns.

Wave 3 of the reference-adoption arc (adapt: terminal-quality success predicate).

* feat(subagents): gate spawn on declarative required MCP servers

A markdown agent's frontmatter can declare required_mcp_servers; spawning that
type via the Agent or RunAgents tools is rejected with a clear message when the
servers are configured-and-absent (after MCP loading settles), instead of
wasting a turn on an agent that cannot reach its tools. While MCP is still
loading the spawn is allowed (servers may yet connect).

Adds required_mcp_servers to AgentTypeDefinition + markdown frontmatter; a
root-only Runtime.mcp_status seam wired from the toolset snapshot; a pure
_missing_required_mcp_servers decision function; and AgentTool.check_required_mcp_servers
reused by both spawn tools. Also fixes a TurnStopReason annotation in the item-8
test that make check surfaced.

Wave 3 of the reference-adoption arc (adapt: required-MCP spawn gate).

* feat(hooks): inject UserPromptSubmit additionalContext into the user turn

A UserPromptSubmit hook could block a prompt but its additionalContext was
ignored. Now a non-blocking hook's additional_context is appended to the user
turn as a system reminder so the model sees it as context for this prompt, via a
pure _user_message_with_hook_context builder. Slash-command parsing keeps reading
only the user's original text, never the appended hook context.

Ships the clean UserPromptSubmit half per the catalog's staging; the PostToolUse
half (await the currently fire-and-forget trigger, gated on has_hooks_for, to
inject its additionalContext) is a documented follow-up.

Wave 3 of the reference-adoption arc (adapt: hook additionalContext feedback).

* docs(tasks): record adoption-arc status + Wave 4 execution notes

Waves 1-3 done (10 items); capture the read-before-write technique, the
project-context refactor's test-pin risk, and the max-output-token cross-package
blocker for Wave 4 so the remaining work is executable cleanly.

* feat(file): stale-overwrite guard via a per-agent file read cache

If the agent read a file and it then changed on disk (user edit or another tool),
a WriteFile overwrite is now rejected so the external change is not clobbered.
ReadFile records the mtime at read; WriteFile rejects an overwrite whose on-disk
mtime is newer than the recorded read, and refreshes the read-state after its own
write (so consecutive edits are never falsely flagged); StrReplaceFile likewise
refreshes after editing. First-contact writes (a file never read) are unaffected.

Adapts the reference's read-before-write/file-state-cache to the stale-detection
half only — a full read-before-write requirement is incompatible with pythinker's
'write without read' contract (would break ordinary regenerate-file flows). The
cache (utils/file_read_cache.py, placed there to avoid a tools<->soul import
cycle) keys on the canonical real path, fails open on stat errors, and is
per-agent. Unit + e2e tests (stale-blocked, allowed-after-read, consecutive).

Wave 4 of the reference-adoption arc (adapt: stale-read detection).

* docs(tasks): record executable plan for the remaining Wave 4 items (12, 13)

Item 13: reverse-engineered the precise finish_reason seam in
PythinkerStreamedMessage -> GenerateResult.truncated -> soul escalation ladder.
Item 12: documented the marginal-value/high-brittleness assessment.

* feat(core): surface a truncation signal on GenerateResult

pythinker-core captured no finish_reason, so a response cut off by the
output-token limit (visible text then cap) was returned as a clean completion —
the agent loop could not tell truncation from a normal stop. Capture the
provider's finish_reason in PythinkerStreamedMessage (both stream and non-stream
paths) and set GenerateResult.truncated when it is 'length'. _generate reads it
optionally (getattr) so other stream implementations are unaffected. MockChatProvider
gains a finish_reason arg for testing.

Stage 1 of the max-output-token recovery ladder (Wave 4, item 13); the soul-side
escalate-then-nudge recovery follows in stage 2.

* feat(loop): recover from output-token truncation with bounded continuation nudges

Stage 2 of item 13. StepResult now carries truncated (propagated from
GenerateResult). When a step is cut off by the output-token limit and makes no
tool call, the soul nudges the model to continue from where it stopped instead of
ending the turn with a half-finished answer reported as complete (C06/C13
truthfulness gap). Bounded by loop_control.max_truncation_recoveries (default 3;
0 disables) per turn so a persistently-truncating model cannot loop forever.
Decision is a pure, parametrized-tested function; integration tests cover the
nudge-and-continue path and the disabled-at-zero path.

Wave 4 of the reference-adoption arc (adapt: max-output-token recovery).

* polish(subagents): harden required-MCP gate diagnostics

Warn (and ignore) a non-list required_mcp_servers frontmatter value, and point a
gated spawn at `pythinker mcp auth <server>` for a configured-but-unauthorized
server in addition to `pythinker mcp add`. Reformat path.py.

Follow-up polish to Wave 3 item 9.

* feat(file): extend the stale-overwrite guard to StrReplaceFile

Wave 4 follow-up hardening for the reference-adoption arc:

- StrReplaceFile now applies the same stale-read guard as WriteFile via a
  shared overwrite_is_stale() helper. Exact old-string matching alone cannot
  catch an external edit that leaves the matched string intact while changing
  the rest of the file, so gate on the recorded read mtime as WriteFile does.
  First-contact edits and a tool's own consecutive edits stay unblocked.
- Pin the custom .githooks (core.hooksPath) case in the dangerous-host-path
  deny-set test and call it out in the changelog.
- Pin the system-prompt CLI in the PyInstaller hiddenimports expectation so the
  one-file build keeps shipping it.
- Assert the UserPromptSubmit hook-context path wraps stdout in the
  untrusted-data envelope and strips invisible smuggling characters.

* docs(tasks): record Wave 4 outcomes (#11/#13 done, #12 architectural no-go)

- #11 and #13 are committed; mark them DONE in the execution status with their
  commit refs and the scope adaptations made.
- #12 (move AGENTS.md to a separate session-start <system-reminder>) is recorded
  as an architectural no-go: AGENTS.md must survive compaction and not truncate,
  and only the system prompt (stored separately, never summarized, 32 KiB budget)
  satisfies both. A seed message is lossily summarized at the first compaction
  (prepare() preserves only the last 2 messages verbatim); a dynamic injection is
  hard-capped at 2048 tokens. Both alternatives would need new compaction-pinning
  or unbudgeted-injection machinery the project's MVC rules forbid, for marginal
  non-reference cache value. Keep AGENTS.md in the system prompt.

* feat(prompt): deliver merged AGENTS.md as a session-start system-reminder preamble

Move the merged AGENTS.md out of the immutable system prompt (system.md §11) into a
session-start, user-role <system-reminder> message that is assembled fresh from runtime
state on every model request and is never written to context.history.

Robustness — this placement is the whole point. AGENTS.md carries the project's
non-negotiable rules, so it must (a) survive compaction and (b) never be truncated. Two
tempting homes fail:
- a persisted seed message is lossily summarized at the first compaction (the compactor
  preserves only the last few messages verbatim);
- a dynamic injection is hard-capped by the 2048-token injection budget.
Assembling it at the step boundary, outside history, sidesteps both: compaction cannot
summarize what it never sees, and the injection budget cannot truncate a non-injection.
Net per-request tokens are unchanged (it simply moves from the system role to a leading
user-role reminder), and the system prompt is now free of per-project content.

- render_agents_md_reminder(builtin_args): authoritative <system-reminder> body, or None.
- _with_agents_md_preamble(history, builtin_args): prepend it, unmutated, ahead of history.
- _step assembles normalize_history(_with_agents_md_preamble(...)).
- system.md §11 explains the separate delivery instead of interpolating the block.
- pythinker system-prompt appends the reminder below a labeled divider so the dump stays
  faithful.

Resolves the agent.py:66 TODO. Closes Wave 4 #12 of the reference-adoption arc.

* fix(core): make output-token truncation a required StreamedMessage signal

GenerateResult.truncated drives the agent loop's truncation recovery, but
finish_reason was only an optional getattr property implemented by the Pythinker
and Mock providers. Every other provider returned None, so a reply cut off by the
output-token limit was silently treated as a clean completion — truncation
recovery never fired for Anthropic, OpenAI-legacy, OpenAI-responses, or Google
(4 of 5 real providers). Silent degradation in an execution-critical path.

Make finish_reason a required member of the StreamedMessage Protocol so no
provider can omit it, and map each provider's native truncation signal onto the
loop's 'length' value:

- Anthropic: stop_reason 'max_tokens' -> 'length' (non-stream + message_delta).
- OpenAI-legacy: native choices[0].finish_reason ('length' already).
- OpenAI-responses: incomplete_details.reason 'max_output_tokens' -> 'length'
  (handles the response.incomplete terminal event).
- Google GenAI: candidate.finish_reason MAX_TOKENS -> 'length'.
- Chaos: delegates to the wrapped stream; Echo/ScriptedEcho never truncate.

_generate now reads stream.finish_reason directly (contract-guaranteed). Adds a
two-sided test pinning finish_reason='stop' -> not truncated so the negative side
can't pass only because the default is falsy.

* fix(file,mcp): harden stale-overwrite detection and the required-MCP startup gate

Two review-driven robustness fixes:

- Stale-overwrite guard now tracks (mtime, size), not mtime alone. An external edit
  within the same filesystem-mtime tick, or one that preserves/restores the mtime
  (touch -r, archive extraction), left mtime unchanged and slipped past the guard;
  comparing size as well catches those size-changing edits. ReadFile records both
  from one stat; Write/StrReplace refresh both after their own write.

- mcp_status_snapshot() is now loading-honest: it returns a loading=True snapshot
  (instead of None) while a deferred MCP startup is queued but has not populated the
  server map yet. None now means only "no MCP configured", so the required-MCP spawn
  gate can no longer mistake the startup window for a genuinely-absent server and
  reject a first-turn subagent. Closes the gap between _missing_required_mcp_servers'
  documented "[] while still loading" contract and its None-snapshot handling.

* feat(auth,providers): add Kimi provider and GLM-5.2 defaults, fix MiniMax usage

- Add Kimi K2.7 Code model and new Kimi Coding Plan provider with Moonshot
  Anthropic-compatible endpoint
- Set GLM-5.2 as default Z.AI model with pinned catalog entry for 1M context
- Fix MiniMax token-plan adapter to read percentage-metered usage and convert
  reset times from milliseconds to seconds
- Enhance UI shell modules to surface new providers and integrate new platform
  catalog entries
- Add comprehensive tests for Moonshot and Z.AI auth, and MiniMax usage adapter

Reference adoption Wave 2 delivery.

* fix(core,file): satisfy the finish_reason contract in test doubles; suppress post-write stat

Adversarial-review follow-ups.

- Making finish_reason a required StreamedMessage member meant the six soul/auth
  test doubles that drive a real generate() (StaticStreamedMessage,
  SequenceStreamedMessage, PartialThenErrorStreamedMessage, and the notifications /
  cumulative-usage doubles) raised AttributeError at _generate's direct
  stream.finish_reason read. Give each the no-op property so the now-required
  contract is satisfied and the suites stay green.

- WriteFile's post-write stat() was the only one of the three file tools not wrapped
  in contextlib.suppress(OSError): a stat hiccup after a successful write fell through
  to the except handler and reported a false write failure (and skipped the cache
  refresh, which could then trip a spurious stale flag on retry). Suppress it, keep
  the success result, and omit the size note when the stat is unavailable — matching
  read.py and replace.py.

* feat(file): reject a full overwrite of a partially-read file

The stale-overwrite guard caught external changes but not the read-completeness
case the deep-scan flagged: the agent reads only part of a large file (a capped or
offset ReadFile), then WriteFile-overwrites it — silently discarding the lines it
never saw, with no external change for the mtime/size guard to catch.

Track read completeness alongside (mtime, size): a ReadFile that started at line 1
and returned every line is complete; a capped/offset read is not. A WriteFile
overwrite of a file whose recorded read was incomplete is now rejected with an
actionable message ("read it in full first"). Scope is deliberately narrow so the
write-without-read contract is preserved: first-contact writes, full-read-then-
overwrite, append, and all StrReplaceFile edits (which preserve unseen bytes) are
unaffected. Completeness is sticky — a full read is not undone by a later partial
re-read of the unchanged bytes — and an overwrite/StrReplace refreshes it to
complete (the agent then knows the whole content), while an append leaves it
incomplete (the prior content is still unseen).

* Revert "feat(file): reject a full overwrite of a partially-read file"

This reverts commit af531ea.

* feat(tui): review findings table, lighter borders, and RunAgents polish

Three visual improvements to the TUI tool-card renderer:

1. **Border reharmonisation** — `border` token updated from the old slate
   blue (#3A506D) to a neutral light grey (#e8ebed); `border_muted` follows
   suit (#2B3A52 → #b8bcc0). Panels and markdown table borders now recede
   instead of competing with accent colours.

2. **RunAgents review findings table** — after a completed RunAgents run
   that includes `code-reviewer`, `security-reviewer`, or `review` subagents,
   a compact findings panel is appended below the agent tree. It parses
   structured severity markers from each reviewer's prose (five forms:
   `[SEVERITY]` bullets, colon-bullets, bold-start, named-severity subsection
   headers, and markdown table rows) and aggregates Critical / High / Medium /
   Low counts with a "Reported by" column listing the contributing agents.
   Unparsed reports are tracked separately so the footer ("Parsed N/M · K
   kept as unparsed prose") communicates parsing confidence explicitly.

3. **RunAgents name-extra label** — agent entries whose `name_extra` field is
   set now render the extra label in bold tool-title style instead of plain
   text.

Parser guards:
- `_RE_SEVERITY_IN_HEADER` uses a `(?!-)` lookahead so "## High-level
  overview" is not classified as a high-severity section.
- Mid-sentence severity words ("not a high-risk change") are never counted.

17 direct unit tests verify exact severity counts, reporters mapping, and
aggregate behaviour. 7 renderer-level integration tests cover end-to-end
rendering. Related border-colour assertions updated in two existing test files.

* feat(tui): standardize welcome banner colors and diff sign spacing

- Directory label: periwinkle accent (#B3B9F4) to match skill/highlight color
- Branch label: teal from statusline palette (#64d2c8) to match bottom bar branch
- Diff signs: remove trailing space after +/- so format is {ln} -{content} not {ln} - {content}
- Update tests: accent token assertion for Directory, spacing assertions for diff output

* fix(core): make Gemini finish_reason sticky once 'length' is captured

A second candidate with a non-MAX_TOKENS finish_reason (e.g. STOP) would
overwrite a previously captured 'length', silently dropping truncation
signal. Guard the assignment so 'length' is never overwritten.

* test(core): add streaming + reverse-order sticky-truncation test coverage

* fix(soul): wire budget-exhausted and stuck-loop messages to shell

Both handoff messages were appended to context but never sent to the
wire, so the interactive shell showed no feedback on budget ceiling
or stuck-loop exits. Add wire_send(TextPart) after each append_message.

* fix(ui): sync dark-theme ptk hex values to TUI token constants

_PROMPT_STYLE_DARK carried six stale slate hexes that diverged from
the current _TUI_TOKENS_DARK border/border_muted values. Update them
and add a parity test that binds the ptk layer to the token constants.

* fix(ui): restore preview line for non-review successful agents

The agent tree renderer only showed preview text on error/failed agents.
Non-review successful agents with a brief or summary_preview now also
show a preview line so completed work is visible in the tree.

* fix(subagents): drop non-string values from required_mcp_servers

YAML non-string values (integers, booleans, null) were coerced to strings
via str(s), creating permanently unsatisfiable MCP server names. Filter
to isinstance(s, str) instead so only real string entries are retained.

* docs(changelog): add Unreleased entries for deep-scan fixes

Five correctness and UX fixes: Gemini sticky truncation, shell wire
handoff for budget/stuck messages, dark-theme ptk hex parity,
non-review agent summary preview, and required_mcp_servers type filter.

* style(memory,tests): satisfy CodeQL implicit-concat and mixed-import nits

- recall.py: make the two wrapped recall-block list entries explicit `+`
  concatenations instead of relying on implicit adjacent-literal concat
  (CodeQL py/implicit-string-concatenation-in-list). Output is byte-identical.
- test_shell_timeout_drift.py: drop the redundant `from pythinker_code.tools.shell`
  import and qualify Shell/MAX_*_TIMEOUT through the already-imported `shell_mod`
  so the module is imported in a single style (CodeQL mixed import).

* fix(config): resolve scoped config for read-only system-prompt dump

pythinker system-prompt fell back to an empty Config() whenever
~/.pythinker/config.toml was absent, silently dropping project/local scoped
settings (e.g. extra_skill_dirs) and printing an inaccurate prompt. Add a
persist=False mode to load_config/_load_scoped that runs the full user→project→
local merge with no disk side effects — no share-dir/lock creation, default
seeding, JSON→TOML migration, or auto-gitignore, and skipping the project-trust
read (trust only gates hooks, which no read-only consumer renders). The default
persist=True path is byte-for-byte unchanged. Regression test covers
no-user-config + project-config-present and asserts no config is seeded.

* fix(soul): re-check spend ceiling after billable compaction

The per-session spend ceiling was checked only at the top of the step loop, but
proactive compact_context() makes a billable LLM call that raises
_session_cost_usd. A session just under max_session_cost_usd could therefore pay
for compaction and still run a full model step before the guard re-fired. Add a
second ceiling check immediately after the proactive compaction (before _step()),
reusing the existing budget_exhausted handoff. Regression test asserts no model
step runs once compaction pushes the session over the ceiling.

* fix(file): re-check staleness after approval to close TOCTOU window

StrReplaceFile and WriteFile validated staleness before the (unbounded) approval
prompt, then wrote the in-memory content wholesale — so an external edit landing
during approval was silently clobbered (exact-string matching ran pre-approval and
gives no write-time protection). Add a second overwrite_is_stale / _reject_if_stale
check immediately after approval, returning the same stale-read error. Regression
tests mutate the file during the approval call and assert the external change
survives.

* style(soul,tests): address CodeRabbit type/hygiene nits

- slash.py: annotate the new accept_edits handler with -> None (public src rule).
- test_pythinkersoul_steer.py: annotate the finish_reason test-double property.
- test_agent_tool.py: replace the hardcoded /tmp agent_file with the in-file
  runtime.subagent_store.root pattern, and split the combined assertion for
  clearer failure messages.

* fix(typo): toggleable, edge_diagnostic

Fix the two repo-wide spellcheck failures flagged by the typos CI gate:
'togglable' -> 'toggleable' in a test comment, and 'ede_diagnostic' ->
'edge_diagnostic' in a planning note.
@elkaix
elkaix deleted the fix/ruff-up042-up047-str-enum branch July 17, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant