feat(core): deepen agent core — request assembly, transactional history, catalogues, toolset characterization - #201
Merged
Merged
Conversation
The "Interrupted by user" acknowledgement only prints after the soul re-raises the cancellation, which first awaits a shielded, disk-first context append (the interrupted-tool marker write). That append hops through the shared thread pool, so under heavy CPU contention the ack can legitimately trail the default 15s read budget and the e2e times out. Give this one wait 45s of headroom so local full-suite runs stay stable without masking a genuine hang. The PTY suite is already skipped on CI, so this only affects loaded local runs.
Subagent path, extend, and system_prompt_path references that resolve outside their spec's directory or the built-in agents directory are now rejected fail-closed instead of being opened or recursively loaded. The markdown agent catalogue no longer reclassifies an unexpected parser error as a harmless invalid-field skip, so only genuinely malformed frontmatter is skipped.
…ning # Conflicts: # CHANGELOG.md # docs/en/release-notes/changelog.md # src/pythinker_code/ui/shell/visualize/_blocks.py
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (12)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThis PR adds deterministic agent and skill catalogues, unified request assembly and injection lifecycle handling, transactional context persistence, MCP/toolset resilience, request telemetry, and a toolset characterization benchmark with CLI support. ChangesRuntime overhaul
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PythinkerSoul
participant SkillCatalog
participant RequestLifecycle
participant RequestAssembler
participant LLM
User->>PythinkerSoul: submit task
PythinkerSoul->>SkillCatalog: project task-relevant skills
PythinkerSoul->>RequestLifecycle: prepare trusted sources
RequestLifecycle->>RequestAssembler: provide source results
RequestAssembler-->>PythinkerSoul: assembled history and manifest
PythinkerSoul->>LLM: execute request
PythinkerSoul->>RequestLifecycle: finalize admitted injections
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Internal superpowers plans/specs/reports live under docs/superpowers/ (gitignored, never linked in the published nav) yet are auto-discovered by VitePress and compiled through the Vue SFC parser. A plan doc using <exact-catalogue-name> token syntax was read as an unclosed HTML tag, failing 'npm run build' with 'Element is missing end tag'. Add srcExclude: ['**/superpowers/**'] so these docs are never fed to the compiler, fixing the class of failure rather than escaping individual angle-bracket tokens.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/core/test_dynamic_injection_hooks.py (1)
207-237: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftConstruct a real
PythinkerSoulinstead of mirroring its private initialization.This harness now manually tracks
_checkpoint_with_user_message,_request_lifecycle, and_notified_context_generations. It can pass while production constructor wiring is broken. Instantiate the real object and mock only the compaction boundary.As per path instructions, tests should validate observable behavior rather than mock internal implementation details.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/core/test_dynamic_injection_hooks.py` around lines 207 - 237, Update the test setup around PythinkerSoul to instantiate it through its real constructor instead of manually assigning private fields such as _checkpoint_with_user_message, _request_lifecycle, and _notified_context_generations. Mock only the compaction boundary and retain the injection-hook assertions, so the test exercises production constructor wiring while validating observable behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/benchmark/toolset_characterization.py`:
- Around line 770-803: Ensure both _measure_advertisement and
_measure_mcp_publication always call cleanup() for their MCP-backed toolsets
from a finally block, including when setup or measurement raises an exception.
Preserve the existing measurement and return behavior while moving teardown out
of the happy-path-only flow.
In `@src/pythinker_code/skill/catalog.py`:
- Around line 99-114: Update the catalog initializer to deep-copy each Skill
once and build both _compatibility_mapping and _skills_by_name from those copied
instances. Ensure exhaustive_mapping() and resolve() cannot expose or retain
references to the original mutable skills, while preserving normalized names and
ordering.
In `@src/pythinker_code/soul/context.py`:
- Around line 279-302: Update the temporary-file cleanup around the fdopen call
in the prompt-writing flow to close the raw descriptor when os.fdopen fails.
Ensure the descriptor is closed exactly when ownership was not transferred to
the file object, while preserving the existing tmp_path cleanup and
BaseExceptionGroup behavior.
- Around line 424-451: Update the _usage and _checkpoint validation in the
surrounding record-processing flow to accept only non-boolean integers that are
non-negative. Apply the checks to usage_token_count before assigning token_count
and to checkpoint_id before calculating next_checkpoint_id; retain the existing
warning, rejection, and continue behavior for invalid records.
In `@src/pythinker_code/soul/dynamic_injection.py`:
- Around line 143-160: Update _legacy_source_result to set
RequestFragment.truncatable based on whether policy.truncation equals
FragmentTruncation.ALLOWED, matching the _provided test helper rather than
hardcoding True.
In `@src/pythinker_code/soul/pythinkersoul.py`:
- Around line 2072-2085: Update _commit_and_finalize so history is not appended
before the fallible _request_lifecycle.finalize operation. Prevalidate or
restructure finalization to make it infallible before persisting, or provide an
atomic commit of history and acknowledgements; ensure any failure cannot leave
persisted history without committed lifecycle state.
- Around line 803-843: Update notify_providers so each provider’s
on_context_compacted callback is awaited with a per-provider timeout before
proceeding to the next provider. Handle timeout and cancellation without
allowing one provider to block compaction or recovery, report the failure
through report_handled_error, and log the provider identity with the existing
warning context. Ensure the outer notification_task cancellation path does not
indefinitely wait for a shielded callback that has exceeded its timeout.
In `@src/pythinker_code/soul/request_lifecycle.py`:
- Around line 160-181: Update _prepare so optional registrations from
_ordered_optional are prepared concurrently with asyncio.gather, while
preserving the existing ordered sequential behavior and early break for required
preparation. Aggregate each _prepare_one result into PreparedSources in the
expected policy, result, and acknowledgement order, and retain per-registration
locking semantics.
- Around line 287-320: Update finalize to report every exception collected from
registration.provider.acknowledge_injections before raising
RequestLifecycleError, rather than silently discarding failures after
failures[0]. Reuse the lifecycle’s established logging mechanism if available,
and preserve the first failure as the raised error’s cause.
In `@tests/core/test_compaction_restore.py`:
- Around line 184-191: Update the compaction-restore tests around the
legacy_clear, legacy_write_prompt, legacy_append, and legacy_usage mocks to stop
asserting that internal mutation methods are unused. Remove those
implementation-coupled mocks and assertions, and instead verify the
transactional outcomes: persisted file bytes, history generation, checkpoint
count, and final history state.
- Around line 374-381: Update the coordination waits in the compaction restore
tests around soul.compact_context(), replacement_entered, and
release_replacement to use one consistent CI-safe timeout instead of indefinite
or 200 ms waits. Ensure release signals are performed in finally blocks, and
explicitly handle timeout and cancellation outcomes so tasks are not left
running.
In `@tests/core/test_context_transactions.py`:
- Around line 23-32: Update the _memory helper to include
context.mutation_generation in the returned state snapshot, alongside the
existing Context fields, so rollback and atomicity assertions detect generation
changes.
In `@tests/core/test_dynamic_injection_hooks.py`:
- Around line 107-145: The test
test_stale_operation_does_not_claim_concurrent_replacement_commit must bound
both stale_entered.wait() and awaiting stale with asyncio.wait_for, and ensure
release_stale.set() runs in a finally block so the blocked task is always
released. Handle timeout and cancellation paths explicitly while preserving the
expected ContextGenerationConflictError assertion and existing generation
checks.
In `@tests/core/test_request_assembly_soul.py`:
- Around line 177-187: Strengthen the assertions in the request assembly test
around captured_history by comparing the complete sequence against the expected
Message objects or their serialized byte representation. Preserve validation of
the assembled prompt content while ensuring “Original question,” message
ordering, and all message parts are verified exactly.
In `@tests/core/test_skill_catalog.py`:
- Around line 407-418: Remove the wall-clock duration collection and median
threshold from the test around catalog.search_with_metrics, while preserving the
deterministic metrics assertions and match-count validation. Move the timing
measurement and 100 ms median requirement into an opt-in benchmark or
environment-specific performance test.
In `@tests/core/test_toolset.py`:
- Around line 653-764: Update the three
tests—test_pre_tool_use_block_survives_telemetry_failure,
test_pre_tool_use_exception_fails_open_and_later_call_recovers, and
test_post_tool_use_failure_isolated_from_result_and_later_call—to stop
monkeypatching the private HookEngine._execute_hooks method. Exercise the
configured hook execution path through a controllable public executor or command
fixture, preserving the existing assertions for telemetry isolation, fail-open
recovery, and post-hook fire-and-forget behavior; alternatively move
failure-policy coverage into dedicated HookEngine tests.
---
Outside diff comments:
In `@tests/core/test_dynamic_injection_hooks.py`:
- Around line 207-237: Update the test setup around PythinkerSoul to instantiate
it through its real constructor instead of manually assigning private fields
such as _checkpoint_with_user_message, _request_lifecycle, and
_notified_context_generations. Mock only the compaction boundary and retain the
injection-hook assertions, so the test exercises production constructor wiring
while validating observable behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23ebef24-9f02-4cea-b2f5-84e264884ab2
⛔ Files ignored due to path filters (16)
docs/en/contributing/toolset-characterization.mdis excluded by!docs/**docs/en/customization/agents.mdis excluded by!docs/**docs/en/reference/slash-commands.mdis excluded by!docs/**docs/en/release-notes/changelog.mdis excluded by!docs/**docs/history/CHANGELOG-pre-0.8.0.mdis excluded by!docs/**docs/superpowers/plans/2026-07-10-agent-core-deepening.mdis excluded by!docs/**docs/superpowers/reports/2026-07-10-toolset-characterization.jsonis excluded by!docs/**docs/superpowers/reports/2026-07-10-toolset-characterization.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-agent-catalogue-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-agent-core-deepening-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-context-transactions-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-request-assembly-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-skill-catalogue-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-toolset-characterization-design.mdis excluded by!docs/**tasks/lessons.mdis excluded by!tasks/**tasks/todo.mdis excluded by!tasks/**
📒 Files selected for processing (75)
.coderabbit.yaml.gitignoreCHANGELOG.mdCLAUDE.mdscripts/benchmark_toolset.pysrc/pythinker_code/agents/default/system.mdsrc/pythinker_code/agentspec.pysrc/pythinker_code/benchmark/toolset_characterization.pysrc/pythinker_code/cli/system_prompt.pysrc/pythinker_code/skill/__init__.pysrc/pythinker_code/skill/catalog.pysrc/pythinker_code/soul/agent.pysrc/pythinker_code/soul/btw.pysrc/pythinker_code/soul/compaction_restore.pysrc/pythinker_code/soul/context.pysrc/pythinker_code/soul/dynamic_injection.pysrc/pythinker_code/soul/dynamic_injections/agent_list.pysrc/pythinker_code/soul/dynamic_injections/model_defense.pysrc/pythinker_code/soul/dynamic_injections/permissions_state.pysrc/pythinker_code/soul/pythinkersoul.pysrc/pythinker_code/soul/request_assembly.pysrc/pythinker_code/soul/request_lifecycle.pysrc/pythinker_code/soul/request_primitives.pysrc/pythinker_code/soul/slash.pysrc/pythinker_code/soul/toolset.pysrc/pythinker_code/subagents/catalogue.pysrc/pythinker_code/subagents/discovery.pysrc/pythinker_code/subagents/runner.pysrc/pythinker_code/telemetry/metrics.pysrc/pythinker_code/tools/agent/__init__.pysrc/pythinker_code/tools/skill/__init__.pysrc/pythinker_code/ui/shell/slash.pysrc/pythinker_code/utils/frontmatter.pytests/conftest.pytests/core/test_agent_catalogue_compat.pytests/core/test_agent_catalogue_markdown.pytests/core/test_agent_catalogue_validation.pytests/core/test_agent_list_injection.pytests/core/test_auth_error_handling.pytests/core/test_compaction_restore.pytests/core/test_context.pytests/core/test_context_pruning.pytests/core/test_context_transactions.pytests/core/test_dynamic_injection_budget.pytests/core/test_dynamic_injection_hooks.pytests/core/test_load_agent.pytests/core/test_mcp_cleanup.pytests/core/test_mcp_lifecycle.pytests/core/test_notifications.pytests/core/test_plan_mode.pytests/core/test_prompt_manifest_slash.pytests/core/test_provider_handoff_contract.pytests/core/test_pythinkersoul_ralph_loop.pytests/core/test_pythinkersoul_retry_recovery.pytests/core/test_pythinkersoul_skill_projection.pytests/core/test_pythinkersoul_slash_commands.pytests/core/test_pythinkersoul_steer.pytests/core/test_pythinkersoul_stuck_loop.pytests/core/test_pythinkersoul_turn_balance.pytests/core/test_request_assembly.pytests/core/test_request_assembly_providers.pytests/core/test_request_assembly_soul.pytests/core/test_runtime_auto_state.pytests/core/test_skill_catalog.pytests/core/test_skills_prompt.pytests/core/test_toolset.pytests/core/test_toolset_characterization.pytests/core/test_toolset_concurrency.pytests/e2e/test_shell_pty_e2e.pytests/fixtures/skill_catalog_recall.jsontests/telemetry/test_instrumentation.pytests/test_installation_docs.pytests/tools/test_skill_tool.pytests/ui_and_conv/test_btw.pytests_e2e/test_wire_protocol.py
💤 Files with no reviewable changes (3)
- .gitignore
- .coderabbit.yaml
- tests/test_installation_docs.py
👮 Files not reviewed due to content moderation or server errors (8)
- src/pythinker_code/utils/frontmatter.py
- tests/core/test_agent_catalogue_validation.py
- src/pythinker_code/subagents/catalogue.py
- tests/core/test_pythinkersoul_turn_balance.py
- src/pythinker_code/agentspec.py
- src/pythinker_code/skill/init.py
- src/pythinker_code/subagents/discovery.py
- src/pythinker_code/soul/agent.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pythinker_code/skill/__init__.py (1)
627-654: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCatch UTF-8 decode failures in both discovery passes.
read_text(encoding="utf-8")can raiseUnicodeDecodeError, but these handlers only catchOSError. A single malformedSKILL.mdwill abort discovery instead of skipping the source withunreadable_skill_source. CatchUnicodeErrorin both the directory and flat-file reads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pythinker_code/skill/__init__.py` around lines 627 - 654, Update both discovery-pass exception handlers around the directory and flat-file SKILL.md reads in the skill discovery logic to catch UnicodeError alongside OSError. Ensure malformed UTF-8 sources are skipped and recorded through _collect_discovery_issue with reason_code "unreadable_skill_source", matching existing unreadable-source handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/agentspec.py`:
- Around line 158-176: Update _resolve_within_agent_roots to return the
canonical path produced by absolute.resolve() after validating it, or revalidate
the path immediately before _load_system_prompt() reads
agent_spec.system_prompt_path. Ensure the path used for reading is the same
resolved path that passed the permitted-roots containment check, preventing a
symlink swap between validation and use.
In `@src/pythinker_code/benchmark/toolset_characterization.py`:
- Around line 236-285: Rename the local `mcp` dictionary in
build_threshold_decisions to avoid shadowing the module-level mcp import, and
update all references to that dictionary in the function, including lifecycle
and cleanup threshold evaluations.
- Around line 764-813: Update _measure_advertisement to always tear down the
Runtime created by Runtime.create, wrapping the benchmark work in a try/finally
and invoking the runtime’s existing shutdown/cleanup method in finally. Apply
the same per-sample cleanup to _measure_mcp_publication, ensuring cleanup runs
on both success and failure before the temporary session directory exits.
In `@src/pythinker_code/soul/agent.py`:
- Around line 600-630: The agent catalogue publication flow around
_publish_agent_catalogue must be transactional with the subsequent agent
initialization steps. Stage catalogue, projection, LaborMarket, and
materialized-directory changes until _load_system_prompt() and tool/plugin/MCP
setup succeed, or snapshot and restore every mutation on failure; ensure retries
do not encounter a non-None runtime.agent_catalogue or stale definitions from a
failed load.
In `@src/pythinker_code/soul/context.py`:
- Around line 55-63: Add the missing return type annotation to
ContextPersistenceError.__init__, declaring it returns None while preserving its
existing parameters and behavior.
- Around line 1033-1045: Update update_token_count to validate token_count
before constructing or serializing the usage record: accept only exact
non-negative integers, rejecting booleans and negative values, consistent with
ContextReplacement validation. Preserve the existing locking, reduction, and
append flow for valid counts.
- Around line 424-451: Update the control-record validation in the usage and
checkpoint branches to accept only exact non-negative integers, rejecting
booleans and negative values before any state changes. In the _usage handling,
validate token_count before assigning or clearing pending state; in the
_checkpoint handling, validate id before computing next_checkpoint_id and ensure
the resulting checkpoint state cannot move backward.
- Around line 1015-1031: Update append_messages to validate every element is a
Message before calling _serialize_context_records, raising an explicit input
error for invalid values. Use _reduce_context_records’ accepted results to
reject any unaccepted record before _append_serialized commits the batch, while
preserving valid-message append behavior.
- Around line 278-302: Update _write_system_prompt_sync to close the descriptor
returned by tempfile.mkstemp when os.fdopen fails before its context manager is
entered. Preserve the existing temporary-file cleanup and error-grouping
behavior, ensuring the descriptor is closed exactly once on all failure paths.
In `@src/pythinker_code/soul/dynamic_injections/model_defense.py`:
- Around line 80-99: Initialize self._prepared_injections in the class
constructor before prepare_injections() can access it, using the empty cache
value expected by the method. Keep the existing prepare_injections() cache and
request lifecycle behavior unchanged.
In `@src/pythinker_code/soul/pythinkersoul.py`:
- Around line 1993-1997: The outcome handling around outcome.status and
outcome.reason_code incorrectly maps FAILED results without a reason code to
_not_applicable_source. Ensure every SkillProjectionStatus.FAILED outcome
returns a failure source, using the existing reason when available and an
appropriate fallback failure reason when it is absent; retain
_not_applicable_source only for non-failure outcomes.
- Around line 2087-2098: Update the cancellation handling around
`_commit_and_finalize()` so `commit_task.result()` does not silently discard
persistence failures. Mirror the existing `notify_history_rebuilt` shielded-task
pattern: if the settled task failed, chain that underlying exception onto the
re-raised `CancelledError`; preserve the `"context_persistence_cancelled"`
manifest update and cancellation behavior when no persistence error occurred.
- Around line 2171-2174: Preserve the original task before auto-compaction and
thread that value through the request assembly flow instead of re-reading it
from self._context.history. Update _assemble_request and the downstream
_skill_request_sources projection to use the pre-compaction task, ensuring the
inserted summary user message cannot replace the original input.
In `@src/pythinker_code/soul/request_lifecycle.py`:
- Around line 466-477: Update failed_manifest() so failed RequestManifest
instances do not copy outcomes or any token counters from prior; initialize
these fields to their empty or zero values while preserving the failed status
and sanitized reason_code.
- Around line 307-320: In the finalization loop of the request lifecycle, move
updates to _committed until after registration.provider.acknowledge_injections
succeeds, so failed acknowledgements remain retryable. Define handling for
partial provider acknowledgement on exceptions, preserving only identities
confirmed by the provider and preventing the next request from returning
ALREADY_SATISFIED for unfinalized work.
In `@src/pythinker_code/soul/toolset.py`:
- Around line 638-651: Add a characterization test in test_mcp_lifecycle.py for
the rollback path around the MCP refresh/disconnect operation: monkeypatch
_publish_connected_mcp_tools to raise during rebuilding, invoke the relevant
refresh_mcp_server or disconnect_mcp_server flow, assert the exception
propagates, and verify both self._tool_dict and runtime.mcp_tools exactly match
their pre-operation contents.
In `@src/pythinker_code/subagents/catalogue.py`:
- Around line 89-105: Remove the redundant _freeze_launch_spec calls from the
entry-construction paths in resolve_agent_catalogue and
_resolve_markdown_source, passing the original launch_spec into each
ResolvedAgentEntry. Keep the unconditional freezing in
ResolvedAgentCatalogue.__post_init__ as the single invariant-enforcement point.
- Around line 222-267: Avoid parsing markdown frontmatter twice in
_resolve_markdown_source by reusing the frontmatter mapping already returned by
parse_frontmatter. Update parse_markdown_agent or add a focused variant to
accept the pre-parsed mapping, then pass it from _resolve_markdown_source while
preserving existing validation and diagnostic behavior.
In `@src/pythinker_code/telemetry/metrics.py`:
- Around line 211-232: The request-assembly histogram attributes in
request_assembly_duration_seconds.record must contain only bounded status/count
dimensions. Remove source_ids, budget_limit, budgeted_admitted_tokens, and
non_budgeted_estimated_tokens from attrs, and preserve the required/optional,
included/omitted, truncated, degraded, and failed count labels; route the
removed per-request values through a separate metric or logging path only if one
already exists.
In `@tests/core/test_skill_catalog.py`:
- Around line 407-418: The default test should not enforce the machine-dependent
statistics.median(durations) < 0.1 latency assertion. Remove the timing-based
gate and retain the deterministic metrics assertions in the
catalog.search_with_metrics loop, or move the latency measurement and threshold
into a separately opt-in benchmark.
---
Outside diff comments:
In `@src/pythinker_code/skill/__init__.py`:
- Around line 627-654: Update both discovery-pass exception handlers around the
directory and flat-file SKILL.md reads in the skill discovery logic to catch
UnicodeError alongside OSError. Ensure malformed UTF-8 sources are skipped and
recorded through _collect_discovery_issue with reason_code
"unreadable_skill_source", matching existing unreadable-source handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23ebef24-9f02-4cea-b2f5-84e264884ab2
⛔ Files ignored due to path filters (16)
docs/en/contributing/toolset-characterization.mdis excluded by!docs/**docs/en/customization/agents.mdis excluded by!docs/**docs/en/reference/slash-commands.mdis excluded by!docs/**docs/en/release-notes/changelog.mdis excluded by!docs/**docs/history/CHANGELOG-pre-0.8.0.mdis excluded by!docs/**docs/superpowers/plans/2026-07-10-agent-core-deepening.mdis excluded by!docs/**docs/superpowers/reports/2026-07-10-toolset-characterization.jsonis excluded by!docs/**docs/superpowers/reports/2026-07-10-toolset-characterization.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-agent-catalogue-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-agent-core-deepening-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-context-transactions-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-request-assembly-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-skill-catalogue-design.mdis excluded by!docs/**docs/superpowers/specs/2026-07-10-toolset-characterization-design.mdis excluded by!docs/**tasks/lessons.mdis excluded by!tasks/**tasks/todo.mdis excluded by!tasks/**
📒 Files selected for processing (75)
.coderabbit.yaml.gitignoreCHANGELOG.mdCLAUDE.mdscripts/benchmark_toolset.pysrc/pythinker_code/agents/default/system.mdsrc/pythinker_code/agentspec.pysrc/pythinker_code/benchmark/toolset_characterization.pysrc/pythinker_code/cli/system_prompt.pysrc/pythinker_code/skill/__init__.pysrc/pythinker_code/skill/catalog.pysrc/pythinker_code/soul/agent.pysrc/pythinker_code/soul/btw.pysrc/pythinker_code/soul/compaction_restore.pysrc/pythinker_code/soul/context.pysrc/pythinker_code/soul/dynamic_injection.pysrc/pythinker_code/soul/dynamic_injections/agent_list.pysrc/pythinker_code/soul/dynamic_injections/model_defense.pysrc/pythinker_code/soul/dynamic_injections/permissions_state.pysrc/pythinker_code/soul/pythinkersoul.pysrc/pythinker_code/soul/request_assembly.pysrc/pythinker_code/soul/request_lifecycle.pysrc/pythinker_code/soul/request_primitives.pysrc/pythinker_code/soul/slash.pysrc/pythinker_code/soul/toolset.pysrc/pythinker_code/subagents/catalogue.pysrc/pythinker_code/subagents/discovery.pysrc/pythinker_code/subagents/runner.pysrc/pythinker_code/telemetry/metrics.pysrc/pythinker_code/tools/agent/__init__.pysrc/pythinker_code/tools/skill/__init__.pysrc/pythinker_code/ui/shell/slash.pysrc/pythinker_code/utils/frontmatter.pytests/conftest.pytests/core/test_agent_catalogue_compat.pytests/core/test_agent_catalogue_markdown.pytests/core/test_agent_catalogue_validation.pytests/core/test_agent_list_injection.pytests/core/test_auth_error_handling.pytests/core/test_compaction_restore.pytests/core/test_context.pytests/core/test_context_pruning.pytests/core/test_context_transactions.pytests/core/test_dynamic_injection_budget.pytests/core/test_dynamic_injection_hooks.pytests/core/test_load_agent.pytests/core/test_mcp_cleanup.pytests/core/test_mcp_lifecycle.pytests/core/test_notifications.pytests/core/test_plan_mode.pytests/core/test_prompt_manifest_slash.pytests/core/test_provider_handoff_contract.pytests/core/test_pythinkersoul_ralph_loop.pytests/core/test_pythinkersoul_retry_recovery.pytests/core/test_pythinkersoul_skill_projection.pytests/core/test_pythinkersoul_slash_commands.pytests/core/test_pythinkersoul_steer.pytests/core/test_pythinkersoul_stuck_loop.pytests/core/test_pythinkersoul_turn_balance.pytests/core/test_request_assembly.pytests/core/test_request_assembly_providers.pytests/core/test_request_assembly_soul.pytests/core/test_runtime_auto_state.pytests/core/test_skill_catalog.pytests/core/test_skills_prompt.pytests/core/test_toolset.pytests/core/test_toolset_characterization.pytests/core/test_toolset_concurrency.pytests/e2e/test_shell_pty_e2e.pytests/fixtures/skill_catalog_recall.jsontests/telemetry/test_instrumentation.pytests/test_installation_docs.pytests/tools/test_skill_tool.pytests/ui_and_conv/test_btw.pytests_e2e/test_wire_protocol.py
💤 Files with no reviewable changes (3)
- .coderabbit.yaml
- tests/test_installation_docs.py
- .gitignore
Address CodeRabbit findings on the agent-core-deepening branch: - context: reject boolean/negative token counts in persisted usage and checkpoint records; validate update_token_count at the boundary; close the temporary system-prompt descriptor when os.fdopen fails. - request_lifecycle: surface every provider acknowledgement failure during finalize instead of discarding all but the first. - pythinkersoul: always record a FAILED skill projection as failed rather than blurring it to not-applicable; surface a commit/finalize error as the cause when persistence is cancelled instead of suppressing it. - dynamic_injection: derive legacy fragment truncatability from the policy. - telemetry: drop unbounded per-request token values from request-assembly metric attributes to avoid time-series cardinality growth. - benchmark: disable LSP in the characterization runtime so a per-sample language-server init task is not leaked. - tests: bound coordination waits with CI-safe timeouts and finally-released events; drop a wall-clock median assertion; build the compaction soul through its real constructor; strengthen request-assembly history checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deepens the agent core across five phases, hardening the seams that assemble agent
requests, discover skills and agents, and rewrite conversation history — each behind
explicit compatibility contracts so future agent-core changes are guarded by tests.
What's included
closed; optional guidance reports sanitized degradation; the new
/prompt-manifestcommand explains the latest request composition without storing raw prompts, user
text, or provenance paths.
memory; compaction, pruning, revert, and clear use atomic replacement with coherent
cancellation and rollback. Concurrent revert conflicts stop after a bounded retry
budget instead of starving. Existing JSONL records and restoration remain compatible.
SkillCatalogwith exhaustiveexact-name resolution, sending only task-relevant candidates to the model within an
8,000-character budget. The exhaustive
Runtime.skillsmapping stays available duringthe compatibility window.
precedence, collision diagnostics, and safe provenance handling. Unknown fields warn
now, become errors next minor; legacy adapters remain through that release.
MCP lifecycle; publication rebuilds preserve the previous MCP registry on
registration failure.
path/extend/system_prompt_pathreferencesthat escape their spec directory are rejected; unexpected parser errors are surfaced
instead of being reclassified as harmless "invalid field" skips.
without exposing top-level HTML comments; activity rows stay visually stable; the
coral shimmer is reserved for the active verb spinner.
Compatibility
Public behavior is preserved. New request-assembly, history, catalogue, and toolset
seams are additive with compatibility contracts and warn-then-error deprecation windows
for the agent-definition adapters. Persisted session/JSONL data and restoration behavior
are unchanged.
Testing
make check-pythinker-code— clean (ruff + pyright + ty).tests/ui_and_conv/test_streaming_content_block.py— 124 passed (validates theorigin/mainmerge resolution for the thinking-preview render path).test_request_assembly*,test_skill_catalog,test_toolset_characterization,test_toolset_concurrency, plus theagents-catalogue and context-transaction tests.
CI runs the full
tests+tests_e2egate on this PR.Notes
origin/main(#200) was merged into this branch; it had independently squash-merged theTUI thinking-preview feature this branch also developed, so the overlap was reconciled in
a single merge commit (kept main's cached render call site — the shipped performance
optimization).
Summary by CodeRabbit
/prompt-manifestcommand to inspect sanitized request-assembly details.