Skip to content

fix: surface valid table names on SQL validation 'table not found' retry#1890

Draft
ghostiee-11 wants to merge 1 commit into
holoviz:mainfrom
ghostiee-11:fix/sql-agent-table-not-found-feedback
Draft

fix: surface valid table names on SQL validation 'table not found' retry#1890
ghostiee-11 wants to merge 1 commit into
holoviz:mainfrom
ghostiee-11:fix/sql-agent-table-not-found-feedback

Conversation

@ghostiee-11

Copy link
Copy Markdown
Collaborator

When the SQL agent generates a query that references a table that doesn't exist (e.g. the source name instead of one of its tables, which the model does for multi-table sources like XArraySQLSource where each variable is its own table), validation fails with "table not found". The retry feedback was just the raw error string, so the model kept making the same mistake and exhausted its retries (RetriesExceededError). This surfaces the source's actual table names in the retry feedback so the model recovers.

Before

table 'UploadedSource000002' not found
  -> retry feedback = raw error -> model guesses again -> RetriesExceededError

After

table 'UploadedSource000002' not found
  -> retry feedback: "Use one of these exact table names, not the source name:
     'CMI_C01', 'DQF_C01', ..."
  -> model selects CMI_C01 -> validates

Surfaced while testing the Arraylake source control (#1889) against a real multi-table GOES XArraySQLSource. Adds two deterministic tests for _validate_sql (no LLM needed).

The SQL agent's retry feedback was just the raw error, so when the model
referenced a non-existent table (e.g. the source name for a multi-table
XArraySQLSource) it kept guessing and exhausted retries. Include the
source's actual table names in the feedback so the retry recovers.
@codecov

codecov Bot commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.66%. Comparing base (7cc94fb) to head (d8b5609).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lumen/tests/ai/test_sql_validation.py 95.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1890      +/-   ##
==========================================
+ Coverage   70.60%   70.66%   +0.05%     
==========================================
  Files         195      196       +1     
  Lines       33630    33675      +45     
==========================================
+ Hits        23746    23795      +49     
+ Misses       9884     9880       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ghostiee-11
ghostiee-11 requested a review from ahuang11 June 20, 2026 08:01
Comment thread lumen/ai/agents/sql.py
# source name instead of one of its tables, common for
# multi-table sources like XArraySQLSource). Surface the
# actual table names so the retry stops guessing.
available = source.get_tables()

@ahuang11 ahuang11 Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if there are over 100 of tables in a database? Won't that consume too much context? I think we may need to do similarity search, perhaps through Metaset?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the ordering. It's already capped at [:50] so context stays bounded, but get_tables() comes back in arbitrary order, so with a lot of tables the slice can drop the one the model actually needs.

The metaset's a better fit, and it's already in context here. get_top_tables() ranks by similarity to the query, so I'll pull the relevant names for that source from it and only fall back to get_tables() when it's empty. Pushing that shortly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to mention it's truncated if it's over 50

@ahuang11
ahuang11 marked this pull request as draft July 8, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants