From 1fd5d887e9652cfd3fede7f396bd3f8822d2b94d Mon Sep 17 00:00:00 2001 From: Simon Strandgaard Date: Sun, 8 Mar 2026 15:38:29 +0100 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20clean=20up=20deduplicate=5Flevers.py?= =?UTF-8?q?=20=E2=80=94=20post=20PR=20#203=20review=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../lever/deduplicate_levers.py | 51 ++++++++----------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/worker_plan/worker_plan_internal/lever/deduplicate_levers.py b/worker_plan/worker_plan_internal/lever/deduplicate_levers.py index 630df7a19..6504f0aa5 100644 --- a/worker_plan/worker_plan_internal/lever/deduplicate_levers.py +++ b/worker_plan/worker_plan_internal/lever/deduplicate_levers.py @@ -4,13 +4,9 @@ PROMPT> python -m worker_plan_internal.lever.deduplicate_levers -PROBLEM: A frequent problem is that the deduplicated levers is an empty list, despite having multiple input levers. -002-11-deduplicated_levers_raw.json -I often see output like this: -"deduplicated_levers": [] -It's never supposed to be an empty list. It's supposed to be a list of multiple levers. I need to fix this. """ from enum import Enum +import functools import json import logging import os @@ -43,9 +39,6 @@ class LeverDecision(BaseModel): classification: Literal["keep", "absorb", "remove"] justification: str -class DeduplicationAnalysis(BaseModel): - decisions: List[LeverDecision] - class InputLever(BaseModel): """Represents a single lever loaded from the initial brainstormed file.""" lever_id: str @@ -76,6 +69,13 @@ def _build_compact_history( ] +def _call_llm(chat_message_list: List[ChatMessage], llm: LLM) -> dict: + """Execute a structured LLM call for a single lever classification.""" + sllm = llm.as_structured_llm(LeverClassificationDecision) + chat_response = sllm.chat(chat_message_list) + return {"chat_response": chat_response, "metadata": dict(llm.metadata)} + + DEDUPLICATE_SYSTEM_PROMPT = """ Evaluate each of the provided strategic levers individually. Classify every lever explicitly into one of: @@ -104,7 +104,7 @@ class DeduplicateLevers: """Holds the results of the deduplication.""" user_prompt: str system_prompt: str - response: DeduplicationAnalysis + response: List[LeverDecision] deduplicated_levers: List[OutputLever] metadata: List[Dict[str, Any]] @@ -165,10 +165,7 @@ def execute(cls, llm_executor: LLMExecutor, project_context: str, raw_levers_lis decision: LeverClassificationDecision | None = None result = None - def execute_function(llm: LLM) -> dict: - sllm = llm.as_structured_llm(LeverClassificationDecision) - chat_response = sllm.chat(chat_message_list) - return {"chat_response": chat_response, "metadata": dict(llm.metadata)} + execute_function = functools.partial(_call_llm, chat_message_list) # First attempt with full conversation history. try: @@ -181,6 +178,7 @@ def execute_function(llm: LLM) -> dict: logger.warning(f"Lever {lever.lever_id}: call failed ({e}). Compacting history and retrying.") chat_message_list = _build_compact_history(system_message_with_context, decisions) chat_message_list.append(ChatMessage(role=MessageRole.USER, content=lever_prompt)) + execute_function = functools.partial(_call_llm, chat_message_list) # Second attempt with compacted history (only reached if first attempt failed). if result is None: @@ -210,6 +208,10 @@ def execute_function(llm: LLM) -> dict: classification=LeverClassification.keep, justification="Classification failed after retries. Keeping this lever to avoid data loss." ) + chat_message_list.append(ChatMessage( + role=MessageRole.ASSISTANT, + content=json.dumps({"classification": decision.classification, "justification": decision.justification}), + )) decisions.append(LeverDecision( lever_id=lever.lever_id, @@ -217,21 +219,12 @@ def execute_function(llm: LLM) -> dict: justification=decision.justification, )) - analysis_result = DeduplicationAnalysis(decisions=decisions) - - # The LLM may have been unable to classify some levers. Missing decisions default to keep. - # Code assembles DeduplicationAnalysis from per-lever results; LLM never sees lever_id in the schema. - # Perform the deduplication. + decisions_by_id = {d.lever_id: d for d in decisions} output_levers = [] for lever in input_levers: - # Find the decision for this lever - decision = None - for decision_item in analysis_result.decisions: - if decision_item.lever_id == lever.lever_id: - decision = decision_item - break - if not decision: + lever_decision = decisions_by_id.get(lever.lever_id) + if not lever_decision: # Missing decision for this lever. Keep it. deduplication_justification = "Missing deduplication justification. Keeping this lever." output_lever = OutputLever( @@ -242,12 +235,12 @@ def execute_function(llm: LLM) -> dict: continue # Check if this is a keeper - if decision.classification != LeverClassification.keep: + if lever_decision.classification != LeverClassification.keep: # This is not a keeper continue # This is a keeper - deduplication_justification = decision.justification.strip() + deduplication_justification = lever_decision.justification.strip() if len(deduplication_justification) == 0: deduplication_justification = "Empty explanation. Keeping this lever." @@ -260,7 +253,7 @@ def execute_function(llm: LLM) -> dict: return cls( user_prompt=levers_json, system_prompt=system_prompt, - response=analysis_result, + response=decisions, deduplicated_levers=output_levers, metadata=metadata_list ) @@ -268,7 +261,7 @@ def execute_function(llm: LLM) -> dict: def to_dict(self, include_response=True, include_deduplicated_levers=True, include_metadata=True, include_system_prompt=True, include_user_prompt=True) -> dict: d = {} if include_response: - d["response"] = self.response.model_dump() + d["response"] = [item.model_dump() for item in self.response] if include_deduplicated_levers: d['deduplicated_levers'] = [lever.model_dump() for lever in self.deduplicated_levers] if include_metadata: From cde38fcaeebb24073665602cd4f35c9d88b6962d Mon Sep 17 00:00:00 2001 From: Simon Strandgaard Date: Sun, 8 Mar 2026 16:05:23 +0100 Subject: [PATCH 2/2] fix: replace functools.partial with closure for LLMExecutor compatibility 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 --- .../worker_plan_internal/lever/deduplicate_levers.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/worker_plan/worker_plan_internal/lever/deduplicate_levers.py b/worker_plan/worker_plan_internal/lever/deduplicate_levers.py index 6504f0aa5..614c86e98 100644 --- a/worker_plan/worker_plan_internal/lever/deduplicate_levers.py +++ b/worker_plan/worker_plan_internal/lever/deduplicate_levers.py @@ -6,7 +6,6 @@ """ from enum import Enum -import functools import json import logging import os @@ -155,6 +154,11 @@ def execute(cls, llm_executor: LLMExecutor, project_context: str, raw_levers_lis ChatMessage(role=MessageRole.SYSTEM, content=system_message_with_context), ] + # Closure captures chat_message_list by variable reference, so rebinding + # after compaction is visible on the next call without redefining the function. + def execute_function(llm: LLM) -> dict: + return _call_llm(chat_message_list, llm) + for lever in input_levers: lever_json = json.dumps(lever.model_dump(), indent=2) lever_prompt = ( @@ -165,8 +169,6 @@ def execute(cls, llm_executor: LLMExecutor, project_context: str, raw_levers_lis decision: LeverClassificationDecision | None = None result = None - execute_function = functools.partial(_call_llm, chat_message_list) - # First attempt with full conversation history. try: result = llm_executor.run(execute_function) @@ -178,7 +180,6 @@ def execute(cls, llm_executor: LLMExecutor, project_context: str, raw_levers_lis logger.warning(f"Lever {lever.lever_id}: call failed ({e}). Compacting history and retrying.") chat_message_list = _build_compact_history(system_message_with_context, decisions) chat_message_list.append(ChatMessage(role=MessageRole.USER, content=lever_prompt)) - execute_function = functools.partial(_call_llm, chat_message_list) # Second attempt with compacted history (only reached if first attempt failed). if result is None: