Conversation
jackwildman
requested review from
CallumMcMahon and
rgambee
and removed request for
CallumMcMahon
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| raise EveryrowError( | ||
| "Expected table result (list of records), but got scalar or null" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Bug: The _extract_merge_breakdown function incorrectly gets merge_breakdown from result.additional_properties instead of accessing it as a direct attribute, result.merge_breakdown.
Severity: HIGH
Suggested Fix
Modify the _extract_merge_breakdown function in src/everyrow/task.py to access merge_breakdown directly from the result object via result.merge_breakdown instead of retrieving it from result.additional_properties. Update the misleading comment that claims it's stored in additional_properties.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/everyrow/task.py#L172
Potential issue: In `src/everyrow/task.py`, the `_extract_merge_breakdown` function
attempts to retrieve `merge_breakdown` data from `result.additional_properties`.
However, the `TaskResultResponse.from_dict` method explicitly removes `merge_breakdown`
from the dictionary before assigning the remaining items to `additional_properties`. The
generated model defines `merge_breakdown` as a direct attribute of `TaskResultResponse`.
Consequently, `_extract_merge_breakdown` will always receive `None` and return an empty
`MergeBreakdown` object, preventing users from seeing the breakdown of how rows were
matched.
Did we get this right? 👍 / 👎 to inform future reviews.
| unmatched_left = cast(list[int], d.pop("unmatched_left", UNSET)) | ||
|
|
||
| unmatched_right = cast(list[int], d.pop("unmatched_right", UNSET)) |
There was a problem hiding this comment.
Bug: The unmatched_left and unmatched_right fields lack runtime validation. If the API returns null, they will be assigned None, causing potential AttributeError crashes downstream.
Severity: MEDIUM
Suggested Fix
Implement a parsing function for unmatched_left and unmatched_right that explicitly checks for None, Unset, and list types before assignment. This will align the implementation with the pattern used for other optional, complex fields in the codebase and prevent type-related runtime errors.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/everyrow/generated/models/merge_breakdown_response.py#L185-L187
Potential issue: In `MergeBreakdownResponse.from_dict`, the `unmatched_left` and
`unmatched_right` fields are populated using `d.pop(...)` and then `cast` to
`list[int]`. However, `cast` provides no runtime validation. If the API returns `null`
for these optional fields, the value will become `None`, violating the field's type
annotation of `list[int] | Unset`. Downstream code expecting a list will then fail with
an `AttributeError` when attempting to iterate over the `None` value. This parsing
approach is inconsistent with other complex fields in the same class, which have proper
validation.
Did we get this right? 👍 / 👎 to inform future reviews.
| [tool.uv.sources] | ||
| everyrow = { path = "../", editable = true } | ||
|
|
There was a problem hiding this comment.
this borked the Claude Desktop setup
The extension could not be installed due to the following error: /Applications/Claude.app/Contents/Helpers/disclaimer exited with code 2: error: Failed to generate package metadata for everyrow==0.2.1 @ editable+../
Caused by: /Users/callum/Library/Application Support/Claude/Claude Extensions does not appear to be a Python project, as neither pyproject.toml nor setup.py are present in the directory
There was a problem hiding this comment.
Does the MCP maybe need to be packaged up first? With uv, my understanding is that the tool.uv.sources is only for dev builds, but I think maybe because we're just running a raw uvx to launch it, then we might be treating it like a dev build.
The simplest thing might be to remove this for now. I'll do this shortly.
There was a problem hiding this comment.
I have this PR open #135
We're not doing anything to launch it, see here for docs
It's part of anthropic's mcpb spec, they handle "running" it. Not sure if there's a way of telling their running how to process the pyproject.toml file, e.g. with --no-sources or something
There was a problem hiding this comment.
Got it. Let's go ahead with the simple thing here then
The `[tool.uv.sources]` entry with a relative editable path breaks Claude Desktop extension installs — the relative `../` path doesn't resolve correctly when the extension is unpacked to ~/Library/Application Support/Claude/Claude Extensions/. The regular PyPI dependency `everyrow>=0.2.1` is sufficient for production. Fixes #120 (comment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The `[tool.uv.sources]` entry with a relative editable path breaks Claude Desktop extension installs — the relative `../` path doesn't resolve correctly when the extension is unpacked to ~/Library/Application Support/Claude/Claude Extensions/. The regular PyPI dependency `everyrow>=0.2.1` is sufficient for production. Fixes #120 (comment) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Regenerate files and incorporate new
whoamiendpoint for verifying logged-in state. Also adds everyrow as an editable dev dependency for the mcp project for local testing and dev