refactor(models): Refine MessageAgentThought SQLAlchemy typing#11
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| tool_call_response.append( | ||
| ToolPromptMessage( | ||
| content=tool_responses.get(tool, agent_thought.observation), | ||
| content=str(tool_inputs.get(tool, agent_thought.observation)), |
There was a problem hiding this comment.
Tool response data replaced with tool input data
High Severity
The ToolPromptMessage content incorrectly uses tool_inputs instead of tool_responses. The code parses tool_responses from agent_thought.observation (lines 478-485) but then mistakenly uses tool_inputs.get(tool, ...) when constructing the response message. This causes agent history to contain tool inputs where tool outputs/responses should be, corrupting the conversation context for subsequent agent interactions.
| answer_unit_price=0, | ||
| answer_price_unit=0, | ||
| answer_unit_price=Decimal("0.001"), | ||
| answer_price_unit=Decimal(0), |
There was a problem hiding this comment.
Answer price unit values are swapped
Medium Severity
The answer_unit_price and answer_price_unit values appear to be swapped. The pattern for message_* fields shows message_unit_price=Decimal(0) and message_price_unit=Decimal("0.001"), but for answer_* it's reversed: answer_unit_price=Decimal("0.001") and answer_price_unit=Decimal(0). This is inconsistent with the model's default of Decimal("0.001") for answer_price_unit and could cause incorrect price calculations.
| thought: Mapped[str | None] = mapped_column(LongText, nullable=True, default=None) | ||
| tool: Mapped[str | None] = mapped_column(LongText, nullable=True, default=None) | ||
| tool_labels_str: Mapped[str] = mapped_column(LongText, nullable=False, default=sa.text("'{}'")) | ||
| tool_meta_str: Mapped[str] = mapped_column(LongText, nullable=False, default=sa.text("'{}'")) |
There was a problem hiding this comment.
SQL text object used as Python dataclass default
Medium Severity
Changing MessageAgentThought from Base to TypeBase (which uses MappedAsDataclass) makes default=sa.text("'{}'") problematic. In dataclass mode, the default parameter provides the Python default value, so tool_labels_str and tool_meta_str would default to a TextClause object instead of the string "{}". The correct pattern (as used in other TypeBase classes like Account) is default="{}", server_default=sa.text("'{}'").
Benchmark PR from agentic-review-benchmarks#11
Note
Modernizes agent thought persistence and improves history reconstruction.
MessageAgentThoughtto extendTypeBasewith typedMappedfields,insert_default/default_factoryIDs, nullable defaults, andDecimalpricing with sensible server defaultscreated_by_role/created_bytyping on the model and usesCreatorUserRoleenum in runnerDecimalprices, null-safe/defensive JSON parsing fortool_input/observation, and clearer tool name handling; constructstool_calls/responses even with empty or malformed payloadsAgentThoughtValidationmodel (pydantic) for pre-persist validation scaffoldWritten by Cursor Bugbot for commit 5209691. Configure here.