Summary
In praisonaiagents/agent/chat_mixin.py, the two longest methods in the package — _chat_impl (sync, ~line 3232 onwards) and _achat_impl (async, ~line 4013 onwards) — carry copy-pasted, non-awaiting fragments (the BEFORE_AGENT hook-input construction and the hook prompt-update loop, plus response-template application). The copies have already drifted, which is exactly the maintenance risk duplication creates. This is a duplicate-logic consolidation: extract the identical fragments into shared helpers while keeping the sync/async method split intact.
Current behaviour
The BEFORE_AGENT block appears in both methods. The prompt-update loop is byte-for-byte identical:
# _chat_impl (~3285) and _achat_impl (~4058) — IDENTICAL
for res in hook_results:
if res.output and res.output.modified_input and "prompt" in res.output.modified_input:
prompt = res.output.modified_input["prompt"]
llm_prompt = self._build_multimodal_prompt(prompt, attachments) if attachments else promptThe BeforeAgentInput(...) construction is duplicated too — and has already diverged:
_chat_impl (~3273): prompt=prompt_str, tools_available=[... for t in self.tools]_achat_impl (~4047): prompt=prompt if isinstance(prompt, str) else str(prompt), tools_available=[... for t in (tools or self.tools)]
So the sync path reports self.tools to BEFORE_AGENT hooks while the async path reports tools or self.tools — an unintended inconsistency that slipped in precisely because the block is maintained in two places. Response-template application (_chat_impl ~3243–3257) is similarly mirrored.
Why it matters
- Maintenance duplication in the two longest methods in the SDK. Any change to hook-input fields or prompt-rewrite semantics must be made twice; the
tools_available divergence shows this has already gone wrong once.
- Consolidating the identical, non-awaiting fragments removes the drift surface without touching the async control flow.
Category
Duplicate
Capability preserved
- Both
chat/achat entry points, the sync and async execution paths, and the BEFORE_AGENT hook contract remain exactly as they are.
- The sync/async split, cancellation-token handling, turn-tracking, and
execute_sync vs await execute dispatch are all retained — only the shared, non-awaiting body is factored out.
- Hook blocking (
is_blocked → return None) and prompt-modification behaviour are unchanged.
Proposed approach
Merge duplicate logic (not the methods). Extract small helpers used by both paths, e.g.:
_build_before_agent_input(prompt_str, tools) → returns the BeforeAgentInput (this also lets the tools_available source be defined once, reconciling the drift as a deliberate choice rather than an accident)._apply_hook_prompt_updates(hook_results, prompt, attachments) → returns the possibly-updated (prompt, llm_prompt).
Each method keeps its own has_hooks guard and its own dispatch line (execute_sync vs await execute); only the identical fragments move into the helpers. Do not merge the two methods' control flow.
Resolution sketch
# chat_mixin.py — shared helper (new)
def _apply_hook_prompt_updates(self, hook_results, prompt, attachments):
for res in hook_results:
if res.output and res.output.modified_input and "prompt" in res.output.modified_input:
prompt = res.output.modified_input["prompt"]
llm_prompt = self._build_multimodal_prompt(prompt, attachments) if attachments else prompt
return prompt, llm_prompt
# _chat_impl (sync) # _achat_impl (async)
hook_results = self._hook_runner.execute_sync(...) hook_results = await self._hook_runner.execute(...)
if self._hook_runner.is_blocked(hook_results): if self._hook_runner.is_blocked(hook_results):
return None return None
prompt, llm_prompt = self._apply_hook_prompt_updates(hook_results, prompt, attachments)Severity
Medium (structural maintenance cost in the two most-edited methods; verified drift already present)
Validation
- Duplication confirmed by reading both regions; the prompt-update loop is identical and the
BeforeAgentInput construction differs only in the two fields noted.
- Fix is capability-preserving: helpers contain no
await; the async method keeps await self._hook_runner.execute(...).
- Must be validated against the existing chat/hook tests (
tests/unit/agent/, hook/middleware suites) and the BEFORE_AGENT hook tests before/after, since it touches the execution hot path.
Keep unchanged
- The separate
_chat_impl / _achat_impl methods and the sync/async boundary.
- Async-only concerns: cancellation token,
_clear_turn_tracking, coroutine handling.
- The
has_hooks fast-path guard (no input is built when no BEFORE_AGENT hook is registered).
- All public
chat/achat signatures and semantics.
Summary
In
praisonaiagents/agent/chat_mixin.py, the two longest methods in the package —_chat_impl(sync, ~line 3232 onwards) and_achat_impl(async, ~line 4013 onwards) — carry copy-pasted, non-awaiting fragments (theBEFORE_AGENThook-input construction and the hook prompt-update loop, plus response-template application). The copies have already drifted, which is exactly the maintenance risk duplication creates. This is a duplicate-logic consolidation: extract the identical fragments into shared helpers while keeping the sync/async method split intact.Current behaviour
The
BEFORE_AGENTblock appears in both methods. The prompt-update loop is byte-for-byte identical:The
BeforeAgentInput(...)construction is duplicated too — and has already diverged:_chat_impl(~3273):prompt=prompt_str,tools_available=[... for t in self.tools]_achat_impl(~4047):prompt=prompt if isinstance(prompt, str) else str(prompt),tools_available=[... for t in (tools or self.tools)]So the sync path reports
self.toolstoBEFORE_AGENThooks while the async path reportstools or self.tools— an unintended inconsistency that slipped in precisely because the block is maintained in two places. Response-template application (_chat_impl~3243–3257) is similarly mirrored.Why it matters
tools_availabledivergence shows this has already gone wrong once.Category
Duplicate
Capability preserved
chat/achatentry points, the sync and async execution paths, and theBEFORE_AGENThook contract remain exactly as they are.execute_syncvsawait executedispatch are all retained — only the shared, non-awaiting body is factored out.is_blocked→ returnNone) and prompt-modification behaviour are unchanged.Proposed approach
Merge duplicate logic (not the methods). Extract small helpers used by both paths, e.g.:
_build_before_agent_input(prompt_str, tools)→ returns theBeforeAgentInput(this also lets thetools_availablesource be defined once, reconciling the drift as a deliberate choice rather than an accident)._apply_hook_prompt_updates(hook_results, prompt, attachments)→ returns the possibly-updated(prompt, llm_prompt).Each method keeps its own
has_hooksguard and its own dispatch line (execute_syncvsawait execute); only the identical fragments move into the helpers. Do not merge the two methods' control flow.Resolution sketch
Severity
Medium (structural maintenance cost in the two most-edited methods; verified drift already present)
Validation
BeforeAgentInputconstruction differs only in the two fields noted.await; the async method keepsawait self._hook_runner.execute(...).tests/unit/agent/, hook/middleware suites) and theBEFORE_AGENThook tests before/after, since it touches the execution hot path.Keep unchanged
_chat_impl/_achat_implmethods and the sync/async boundary._clear_turn_tracking, coroutine handling.has_hooksfast-path guard (no input is built when noBEFORE_AGENThook is registered).chat/achatsignatures and semantics.