Replace curl-download with paginated inline results + widget JSON endpoint

[RafaelPo]

Summary

• Replace curl-download mode with paginated inline results (default 50 rows in LLM context)
• Add ?format=json to download endpoint so the widget can fetch the full dataset independently via results_url
• Tool docstring instructs the agent to ask the user how many rows to ingest before calling
• Fix docker-compose.local.yaml to use variable substitution for MCP_SERVER_URL (was hardcoded, breaking tunnel setups)
• Remove curl_mode, _build_curl_result_response, curl_preview_rows setting, and Redis-unavailable fallback

Test plan

• 366 unit tests pass
• E2E: connect via Claude Desktop, run a task, verify agent asks about row count and widget shows full data

🤖 Generated with Claude Code

[RafaelPo]

@claude code review

[sentry]

Bug: The function try_store_result incorrectly returns None when its type hint specifies list[TextContent], and the caller does not handle the None case.
_{Severity: HIGH}

Suggested Fix

Update try_store_result to handle the expired token case without returning None. For example, it could raise an appropriate HTTP exception or return an empty list. Alternatively, change the function's return type hint to list[TextContent] | None and ensure the calling function everyrow_results_http correctly handles the None case.

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: everyrow-mcp/src/everyrow_mcp/tools.py#L925-L932

Potential issue: The function `try_store_result` is type-hinted to return
`list[TextContent]`, but it can return `None` if a poll token has expired (typically
after 24 hours). The calling function, `everyrow_results_http`, does not handle the
`None` case and returns it directly. This mismatch between the declared return type and
the actual return value will lead to a runtime error when a client attempts to retrieve
results for a task whose poll token has expired, breaking the results retrieval flow for
older tasks.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[RafaelPo]

@claude code review

[github-actions]

Claude finished @RafaelPo's task in 1m 51s —— View job

---

Code Review

• Read CLAUDE.md and repo guidelines
• Review PR diff and changed files
• Evaluate Sentry bot findings
• Post review feedback

---

Clean PR — the architecture shift from curl-download to paginated inline results + widget JSON endpoint is well-motivated and the test coverage is solid. A few items worth addressing:

---

1. Unhandled exception from try_store_result in everyrow_results_http (High)

tools.py:927-933 — try_store_result can now raise (Redis failure or poll-token expiration), but the caller has no try/except around it. Both Sentry findings point at this. The other code paths in everyrow_results_http (ownership check, API fetch) are all wrapped in try/except with user-friendly TextContent error messages — this one should follow the same pattern.

I agree that removing the silent inline-results fallback is a reasonable design choice (Redis is a required dependency), but the error should still be caught and returned as a TextContent error message rather than propagating as an unhandled exception through the MCP framework.

# tools.py:926-934 — suggested change
try:
    return await try_store_result(
        task_id, df, params.offset, params.page_size,
        session_url, mcp_server_url=mcp_server_url,
    )
except Exception:
    logger.exception("Failed to store/retrieve results for task %s", task_id)
    return [
        TextContent(
            type="text",
            text=f"Error preparing results for task {task_id}. Please try again.",
        )
    ]

Fix this →

---

2. Download token consumed before format validation (Low)

routes.py:271-301 — The download token is atomically popped (consumed, single-use) on line 271, but format validation happens on line 297. If a client passes ?format=xml, the token is wasted and they get a 400. Not a security issue (widget always sends format=json), but moving the format check before the token pop would be more defensive:

# Validate format before consuming the token
fmt = request.query_params.get("format", "csv")
if fmt not in ("csv", "json"):
    return JSONResponse(
        {"error": "Unsupported format"}, status_code=400, headers=cors
    )

# Atomically consume the download token
token_task_id, remaining_ttl = (
    await redis_store.pop_download_token(provided) if provided else (None, 0)
)

Fix this →

---

3. _sanitize_records is now a cross-module public API (Nit)

routes.py:19 — _sanitize_records (underscore-prefixed, private) is now imported by routes.py. Consider renaming it to sanitize_records to reflect its actual visibility, or move it to a shared utils module.

---

What looks good

• Widget-first architecture: Having the widget mint fresh download tokens on demand (fetchFullResultsWithFreshToken) instead of baking URLs into the tool result is a solid improvement — eliminates stale-token issues on re-render.
• Widget JSON only on first page (offset=0): Smart optimization with a well-documented design note explaining the trade-off.
• Memory guard on JSON conversion (routes.py:305-320): Good defensive boundary with the max_json_size check.
• docker-compose.local.yaml fix: Variable substitution for MCP_SERVER_URL — small but important for tunnel setups.
• Test coverage: Good coverage of the new JSON format endpoint, fetch_full_results widget flag, and the Redis failure behavior change.

---

| Branch: feat/curl-download-results

[sentry]

Bug: The code compares the character count of csv_text using len() against max_json_size, a limit in bytes. This can underestimate the true memory size for multi-byte UTF-8 characters.
_{Severity: MEDIUM}

Suggested Fix

The check should compare the byte length of the string, not the character count. Encode the string to bytes before checking its length: if len(csv_text.encode('utf-8')) > max_json_size:.

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: everyrow-mcp/src/everyrow_mcp/routes.py#L308-L309

Potential issue: A check intended to prevent memory exhaustion compares the character
count of `csv_text` with a byte-based limit, `max_json_size`. Since `len()` on a string
returns the number of characters, not bytes, this check is inaccurate for data
containing multi-byte UTF-8 characters (e.g., accented or CJK characters). A string that
passes the character count check could be significantly larger in bytes, potentially
bypassing the intended memory limit and leading to excessive memory usage or
Out-of-Memory errors during the subsequent JSON conversion.