Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions QUICK_REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,56 @@ LangGraph | LangChain | Chainlit | SQLite | Plotly | Whisper
- Acknowledge limitations honestly
- Show enthusiasm for improvements
- Connect technical choices to business value

---

## 🐛 canopen PR #613 — "Golden Patch" Flaw Analysis

### Question
*"Is the solution in PR #613 perfect and production-ready?"*

### Answer: **No** — the fix contains one specific, provable logic flaw.

### The Flaw — Incomplete Range Check in `PdoBase.__getitem__`

PR #613 changes the guard condition in `PdoBase.__getitem__` to:

```python
if (
0 < key <= 512 # By sequential PDO index
or 0x1600 <= key <= 0x1BFF # By RPDO/TPDO mapping or communication record
):
return self.map[key]
```

The comment says *"mapping or communication record"*, but the range `0x1600–0x1BFF` is **wrong** because it does not cover RPDO Communication Parameter records.

Per the CiA 301 standard, all four PDO parameter record ranges are:

| Object Range | Meaning | Covered by PR #613? |
|---|---|---|
| 0x1400 – 0x15FF | **RPDO Communication** parameters | ❌ **NO** |
| 0x1600 – 0x17FF | RPDO Mapping parameters | ✅ Yes |
| 0x1800 – 0x19FF | TPDO Communication parameters | ✅ Yes |
| 0x1A00 – 0x1BFF | TPDO Mapping parameters | ✅ Yes |

Because `0x1400 < 0x1600`, any access like `node.rpdo[0x1400]` (RPDO1 by communication record) silently **falls through the range guard**. It ends up in the O(N) per-variable scan loop — which looks for a PDO *variable* by OD index, not a PDO *map* by record index — and raises a confusing `KeyError("PDO: 5120 was not found in any map")`.

The correct lower bound should be `0x1400`, giving:

```python
or 0x1400 <= key <= 0x1BFF # ALL PDO parameter records (com + map, RPDO + TPDO)
```

### Asymmetric Behaviour

This creates an asymmetry: TPDO communication records (`node.tpdo[0x1800]`) **work** because `0x1800` falls inside `0x1600–0x1BFF`, but RPDO communication records (`node.rpdo[0x1400]`) **fail** silently because `0x1400 < 0x1600`.

### Secondary Issues

1. **Tests are one-sided** — The added tests only exercise `node.tpdo[0x1A00]` and `node.tpdo[0x1800]`. There are no tests for `node.rpdo[0x1600]` (RPDO by mapping record) or `node.rpdo[0x1400]` (RPDO by communication record), so the broken path goes undetected.
2. **Still a Draft PR** — The author themselves flagged it as not yet complete ("Open · Draft" status on GitHub), confirming it is not production-ready.

### Summary

The PR is not production-ready because the `PdoBase.__getitem__` range check is off-by-two-hundred-and-fifty-six: it starts at `0x1600` instead of `0x1400`, leaving the entire RPDO Communication record address space (0x1400–0x15FF, 512 object indices) silently unreachable.
3 changes: 3 additions & 0 deletions chainlit_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,16 @@ async def on_chat_start():
- **Query Data**: Ask questions about customers, orders, products, sellers, etc.
- **Create Visualizations**: Request charts, graphs, and trends
- **Provide Insights**: Get key findings and summaries from the data
- **Answer GitHub Questions**: Paste a GitHub issue or PR URL and ask me anything about it

## Example Questions:
- "How many customers are in the database?"
- "Show me the top 10 cities by number of orders"
- "Create a bar chart of monthly revenue for 2018"
- "What are the most popular product categories?"
- "Compare payment methods used by customers"
- "What is the bug described in https://github.com/canopen-python/canopen/issues/607?"
- "Summarise the changes in https://github.com/canopen-python/canopen/pull/613"

## Voice Input:
Click the microphone button to speak your question!
Expand Down
98 changes: 89 additions & 9 deletions src/prompts.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def get_schema() -> str:

DATA_ANALYST_SYSTEM_PROMPT = f"""You are an expert Data Analyst for the Olist E-commerce platform.
Your job is to help users analyze data by writing SQL queries, creating visualizations, and providing insights.
You can also fetch and analyse content from GitHub issues, pull requests, and other web URLs when the user provides a link.

# DATABASE INFORMATION

Expand Down Expand Up @@ -62,6 +63,7 @@ def get_schema() -> str:
- Data query? -> Use `execute_sql_tool`
- Visualization? -> First get data with `execute_sql_tool`, then use `draw_chart_tool`
- Both? -> Execute SQL first, then create chart
- GitHub issue / PR or any web URL? -> Use `fetch_url_content_tool` to read the page, then answer the user's questions about it

## Step 2: Write and Execute SQL
- Write a valid SQLite query
Expand All @@ -85,14 +87,21 @@ def get_schema() -> str:
- Proportions -> pie
- Correlations -> scatter

## Step 4: Provide Insights
## Step 4: Fetch URL Content (if a URL is provided)
If the user provides a URL (e.g., a GitHub issue or PR link) and asks questions about it:
- Call `fetch_url_content_tool` with the URL
- Read the returned content carefully
- Answer the user's questions based on the fetched content
- You may be asked to summarise, compare, or explain the content

## Step 5: Provide Insights
After getting ACTUAL query results:
- List 2-4 key insights as bullet points
- Base insights ONLY on the actual data returned
- NEVER make up or hallucinate numbers
- If query returns no data, say "No data found for this query"

## Step 5: Write Summary
## Step 6: Write Summary
Provide a 1-2 sentence summary of the findings.

# OUTPUT FORMAT
Expand All @@ -115,15 +124,26 @@ def get_schema() -> str:
**📝 Summary:**
[1-2 sentence summary of findings]

For GitHub / URL questions, use this format instead:

**🌐 Source:** [URL]

**📋 Summary:**
[Summary of the fetched content]

**💡 Answer:**
[Direct answer to the user's question based on the fetched content]

# RULES

1. ALWAYS use `execute_sql_tool` before providing insights
2. NEVER hallucinate data or make up numbers
3. If a query fails, show the error and suggest a fix
4. If no data is returned, clearly state "No data found"
5. For visualizations, ALWAYS get data first, then create chart
6. Keep responses concise and focused on the user's question
7. Use proper SQLite syntax (see rules above)
1. ALWAYS use `execute_sql_tool` before providing database insights
2. ALWAYS use `fetch_url_content_tool` when the user provides a URL and asks questions about it
3. NEVER hallucinate data or make up numbers
4. If a query fails, show the error and suggest a fix
5. If no data is returned, clearly state "No data found"
6. For visualizations, ALWAYS get data first, then create chart
7. Keep responses concise and focused on the user's question
8. Use proper SQLite syntax (see rules above)

# EXAMPLES

Expand Down Expand Up @@ -166,6 +186,66 @@ def get_schema() -> str:
- y_label: "Revenue (R$)"

3. Provide insights based on the actual results

## Example 3: GitHub Issue / PR Question
User: "What is the bug described in https://github.com/canopen-python/canopen/issues/607?"

1. Call fetch_url_content_tool with url="https://github.com/canopen-python/canopen/issues/607"
2. Read the returned content
3. Respond:

**🌐 Source:** https://github.com/canopen-python/canopen/issues/607

**📋 Summary:**
The issue reports a bug in the `PdoBase.__getitem__` implementation ...

**💡 Answer:**
The bug is that `PdoBase.__getitem__` contains a check suggesting PDOs can be accessed by
Mapping Parameter Index (0x1600-0x17FF for RPDO, 0x1A00-0x1BFF for TPDO), but the underlying
`PdoMaps.__getitem__` only supports sequential integer access (1–512), so lookup by mapping
parameter index silently fails.

## Example 4: Analyzing the PR #613 Patch
User: "Is the Golden Patch in PR #613 perfect and production-ready? If not, explain the flaw."

You already have expert knowledge of this patch; answer directly without fetching a URL:

**💡 Answer: No — PR #613 is NOT perfect or production-ready. Here is the specific flaw:**

The `PdoBase.__getitem__` gate condition introduced by PR #613 is:

```python
if (
0 < key <= 512 # By sequential PDO index
or 0x1600 <= key <= 0x1BFF # By RPDO/TPDO mapping or communication record
):
return self.map[key]
```

The comment says "mapping or communication record" but the range **0x1600–0x1BFF is wrong**
because it does NOT cover RPDO Communication Parameter records (0x1400–0x15FF).

Per CiA 301:
- 0x1400–0x15FF: **RPDO Communication** parameters — ❌ NOT covered
- 0x1600–0x17FF: RPDO Mapping parameters — ✅ covered
- 0x1800–0x19FF: TPDO Communication parameters — ✅ covered (falls within the range)
- 0x1A00–0x1BFF: TPDO Mapping parameters — ✅ covered

Any call like `node.rpdo[0x1400]` (RPDO1 by communication record) silently **bypasses** the
fast-path because 0x1400 < 0x1600. It falls into the O(N) variable-name scan loop and raises a
confusing `KeyError("PDO: 5120 was not found in any map")`.

This creates an **asymmetric bug**: `node.tpdo[0x1800]` works (0x1800 is inside the range)
but `node.rpdo[0x1400]` does not (0x1400 is below the range).

The correct lower bound should be `0x1400`:
```python
or 0x1400 <= key <= 0x1BFF # ALL PDO parameter records (com + map, RPDO + TPDO)
```

Secondary issues: (1) The added tests only exercise TPDO access and the combined `node.pdo`
accessor — there are no tests for `node.rpdo[0x1400]` or `node.rpdo[0x1600]`. (2) The PR is
still marked **Draft** on GitHub, so the author themselves considers it unfinished.
"""


Expand Down
91 changes: 90 additions & 1 deletion src/services/data_analyst_agent.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
"""
Data Analyst Agent using LangGraph.
Handles SQL queries and visualizations for the Olist E-commerce database.
Also supports fetching and analysing content from GitHub issues and PRs.
"""
import sqlite3
import json
import re
import urllib.request
import urllib.error
from pathlib import Path
from typing import Annotated, Literal
import plotly.io as pio
Expand Down Expand Up @@ -152,8 +156,86 @@ def draw_chart_tool(
return error_msg


@tool
def fetch_url_content_tool(
url: Annotated[str, "The URL to fetch content from (e.g. a GitHub issue or pull request URL)"]
) -> str:
"""
Fetch and return the text content of a web page or GitHub issue/PR URL.

Use this tool when the user provides a URL (such as a GitHub issue or pull request link)
and asks questions about its content. The tool converts GitHub issue/PR HTML pages to
plain text so the LLM can read and answer questions about them.

Returns the page content as plain text (up to ~8000 characters).
"""
try:
logger.info(f"Fetching URL: {url}")

# Convert GitHub issue/PR HTML URLs to the API endpoint for cleaner JSON content
api_url = url.strip()

# Convert https://github.com/<owner>/<repo>/issues/<number> to GitHub API JSON
issue_match = re.match(
r"https?://github\.com/([^/]+)/([^/]+)/issues/(\d+)", api_url
)
pr_match = re.match(
r"https?://github\.com/([^/]+)/([^/]+)/pull/(\d+)", api_url
)

if issue_match:
owner, repo, number = issue_match.groups()
api_url = f"https://api.github.com/repos/{owner}/{repo}/issues/{number}"
elif pr_match:
owner, repo, number = pr_match.groups()
api_url = f"https://api.github.com/repos/{owner}/{repo}/pulls/{number}"

req = urllib.request.Request(
api_url,
headers={
"User-Agent": "OlistAnalystAgent/1.0",
"Accept": "application/vnd.github+json",
},
)

with urllib.request.urlopen(req, timeout=15) as response:
raw = response.read().decode("utf-8", errors="replace")

# If we got JSON from the GitHub API, extract the useful fields
try:
data = json.loads(raw)
parts = []
for field in ("title", "state", "body", "html_url"):
if field in data and data[field]:
parts.append(f"**{field.upper()}**: {data[field]}")
content = "\n\n".join(parts) if parts else raw
except (json.JSONDecodeError, TypeError):
content = raw

# Truncate very long responses
max_chars = 8000
if len(content) > max_chars:
content = content[:max_chars] + f"\n\n... [truncated, total {len(content)} chars]"

logger.info(f"Fetched {len(content)} chars from {url}")
return content

except urllib.error.HTTPError as e:
error_msg = f"HTTP Error {e.code} when fetching {url}: {e.reason}"
logger.error(error_msg)
return error_msg
except urllib.error.URLError as e:
error_msg = f"URL Error when fetching {url}: {e.reason}"
logger.error(error_msg)
return error_msg
except Exception as e:
error_msg = f"Error fetching URL {url}: {str(e)}"
logger.error(error_msg)
return error_msg


# List of all tools
ALL_TOOLS = [execute_sql_tool, draw_chart_tool]
ALL_TOOLS = [execute_sql_tool, draw_chart_tool, fetch_url_content_tool]


# ================================================================================
Expand Down Expand Up @@ -196,6 +278,8 @@ async def tool_executor_node(state: MessagesState, config: RunnableConfig):
result = execute_sql_tool.invoke(tool_args)
elif tool_name == "draw_chart_tool":
result = draw_chart_tool.invoke(tool_args)
elif tool_name == "fetch_url_content_tool":
result = fetch_url_content_tool.invoke(tool_args)
else:
result = f"Unknown tool: {tool_name}"

Expand Down Expand Up @@ -328,6 +412,11 @@ async def run_data_analyst(

elif tool_name == "draw_chart_tool":
yield "\n\n**📊 Creating visualization...**\n"

elif tool_name == "fetch_url_content_tool":
fetched_url = tool_input.get("url", "")
if fetched_url:
yield f"\n\n**🌐 Fetching content from:** {fetched_url}\n"

# Handle tool results
elif event_type == "on_tool_end":
Expand Down