fix(test): repair stale test_delegate_compactor_safety.py on live-config - #52
Conversation
Two failures the brief named (add task_count=1 at both _build_child_agent call sites; set compression_enabled on the compaction-path mocks) plus the deeper cause: the delegation shape-mirror feature these tests validated (_delegation_shape_snapshot / protect_first_n bump / _frozen_system_prompt reuse) was removed from the source in the upstream sync on live-config. Removed the orphaned TestProtectFirstNBump class and the orphaned test_frozen_prompt_reused_after_compaction; kept the still-valid rebuild-on-compaction assertion. File is now green. Sibling tests/tools/test_delegate_shape_mirror.py is orphaned the same way (tracked separately). Patch note: ~/.hermes/plans/hermes-patches/delegate-compactor-test-task-count.md
🔎 Lint report:
|
| Rule | Count |
|---|---|
PLW1514 |
1 |
First entries
gateway/run.py:5594: [PLW1514] `open` in text mode without explicit `encoding` argument
✅ Fixed issues (1):
| Rule | Count |
|---|---|
PLW1514 |
1 |
First entries
../../../../../tmp/lint-base/gateway/run.py:5594: [PLW1514] `open` in text mode without explicit `encoding` argument
Unchanged: 0 pre-existing issues carried over.
ty (type checker)
Total: 11571 on HEAD, 11572 on base (✅ -1)
🆕 New issues (2):
| Rule | Count |
|---|---|
unresolved-attribute |
2 |
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`
run_agent.py:3026: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
✅ Fixed issues (3):
| Rule | Count |
|---|---|
missing-argument |
1 |
invalid-argument-type |
1 |
invalid-assignment |
1 |
First entries
tests/tools/test_delegate_compactor_safety.py:156: [missing-argument] missing-argument: No argument provided for required parameter `task_count` of function `_build_child_agent`
tests/tools/test_delegate_compactor_safety.py:209: [invalid-argument-type] invalid-argument-type: Argument to function `AIAgent._compress_context` is incorrect: Expected `str`, found `None`
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`
Unchanged: 6090 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request cleans up tests/tools/test_delegate_compactor_safety.py by removing obsolete tests for delegation shape mirror invariants that were removed in an upstream sync. It introduces a new test class, TestCompressContextRebuildsPrompt, to verify that the system prompt is rebuilt on compaction. Feedback on this change suggests setting agent.compression_enabled to False in the unit test to prevent executing unnecessary auxiliary client resolution logic, which makes the test more robust and isolated.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| agent = MagicMock(spec=AIAgent) | ||
| agent._frozen_system_prompt = None # ← not a child | ||
| agent.compression_enabled = True |
There was a problem hiding this comment.
Setting agent.compression_enabled = True causes check_compression_model_feasibility to run its full auxiliary client resolution logic, which is unnecessary for this unit test and could make it fragile if the environment or configuration changes. Setting it to False allows the feasibility check to return immediately, making the test more robust, isolated, and faster.
| agent.compression_enabled = True | |
| agent.compression_enabled = False |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Make the compactor mock take the success path
Because context_compressor is a bare MagicMock, getattr(agent.context_compressor, "_last_compress_aborted", False) in compress_context returns a truthy child mock. This test therefore exercises the compression-aborted fallback and returns the original messages; _build_system_prompt is still called there, so the assertion passes without covering the successful compaction path it claims to verify. Set _last_compress_aborted = False (and preferably the related error fields to None) on the fake compressor before invoking _compress_context.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """Compaction invokes _build_system_prompt and returns its result.""" | ||
| from run_agent import AIAgent | ||
|
|
||
| agent = MagicMock(spec=AIAgent) |
There was a problem hiding this comment.
tools on the compression mock
When this test runs with the normal dependencies installed, the mocked compression path reaches agent.conversation_compression.compress_context's final token estimate, which directly evaluates agent.tools or None. MagicMock(spec=AIAgent) does not expose instance attributes like tools unless the fixture sets them, so this test still raises AttributeError: Mock object has no attribute 'tools' instead of proving the prompt rebuild behavior. Add agent.tools = [] (or avoid a strict spec for instance state) before calling _compress_context.
Useful? React with 👍 / 👎.
1 participant
What
Repairs the pre-existing RED test
tests/tools/test_delegate_compactor_safety.pyon thelive-configbase. Every fork PR inherits this failure (#46/#47/#48 test slices 1-5,8 were RED on it).Two layers of staleness
1. The two failures called out in the brief (mechanical):
_build_child_agent()on live-config now requires a positionaltask_count: int(tools/delegate_tool.py). The test called it without that arg at both call sites →TypeError.agent/conversation_compression.py:check_compression_model_feasibility, which readsagent.compression_enabled. TheMagicMock(spec=AIAgent)agents lacked it →AttributeError.2. The deeper cause (orphaned feature tests):
After the two mechanical fixes, 2 of 5 tests still failed because they assert invariants of a feature that was removed from the source in the upstream sync that landed on live-config (
293a237fd Merge origin/main into live-config (upstream sync: 107 commits)):tools/delegate_tool.pyno longer touches the child compactor (protect_first_n,_delegation_shape_snapshot,messages_prefix,context_compressor).run_agent.pyno longer definescapture_delegation_snapshotor_frozen_system_prompt.agent/conversation_compression.pyunconditionally rebuilds the system prompt — no frozen-prompt reuse branch.So
TestProtectFirstNBump(whole class) andtest_frozen_prompt_reused_after_compactionwere orphaned. Removed them; kept the still-valid rebuild-on-compaction assertion (test_no_frozen_prompt_means_rebuild_happens, renamedtest_rebuild_happens_on_compaction).Verification
(run via the shared venv:
venv/bin/python -m pytest …)Out of scope (follow-up)
tests/tools/test_delegate_shape_mirror.pyis orphaned the same way (11/12 RED) — tracked as a separate cleanup card so this PR stays scoped to the file the task named.Patch note:
~/.hermes/plans/hermes-patches/delegate-compactor-test-task-count.md. Do not merge — Eric merges.