Skip to content

fix: clean up deduplicate_levers.py — post PR #203 review fixes - #206

Merged
neoneye merged 2 commits into
mainfrom
fix/deduplicate-levers-review-cleanup
Mar 8, 2026
Merged

neoneye merged 2 commits into
mainfrom
fix/deduplicate-levers-review-cleanup

Conversation

@neoneye

@neoneye neoneye commented Mar 8, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix broken USER→ASSISTANT alternation: When both LLM attempts fail, the default keep decision now appends a synthetic ASSISTANT message, preventing USER→USER sequences that break strict LLM APIs
  • Extract closure to module-level function: _call_llm defined once at module level; loop uses functools.partial to bind chat_message_list
  • Dict lookup for decisions: Replaced O(n²) inner loop with decisions_by_id dict for output assembly
  • Remove DeduplicationAnalysis wrapper: Class only used internally; response is now List[LeverDecision] directly
  • Remove stale docstring: The PROBLEM: block described a bug already fixed in PR fix: DeduplicateLeversTask — per-lever decomposition, growing conversation, compact fallback #203
  • Rename reused variable: Output loop uses lever_decision to avoid shadowing the per-lever decision (different types)

Follows up on PR #203 code review. The Enum/Literal parity issue was already fixed in PR #205.

Test plan

  • pytest worker_plan/worker_plan_internal/tests/test_enum_literal_parity.py -v — all 14 tests pass
  • py_compile syntax check passes
  • Verify pipeline runs end-to-end (requires LLM backend)

🤖 Generated with Claude Code

neoneye and others added 2 commits March 8, 2026 15:38
- Fix broken USER→ASSISTANT alternation when default keep is used
- Extract closure to module-level _call_llm with functools.partial
- Use dict lookup for decisions instead of O(n²) loop
- Remove redundant DeduplicationAnalysis wrapper class
- Remove stale PROBLEM docstring (already fixed in PR #203)
- Rename reused `decision` variable to `lever_decision` in output loop

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lity

functools.partial objects don't support typing.get_type_hints(), which
LLMExecutor._validate_execute_function() calls. This caused every LLM
call to raise TypeError, silently caught by the retry logic, resulting
in all levers defaulting to "keep" with no actual deduplication.

Fix: use a closure defined once before the loop that delegates to
_call_llm. The closure captures chat_message_list by variable reference,
so rebinding after compaction is visible without redefining the function.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@neoneye
neoneye merged commit d457826 into main Mar 8, 2026
3 checks passed
@neoneye
neoneye deleted the fix/deduplicate-levers-review-cleanup branch March 8, 2026 15:32
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