add changes - #27
Conversation
📝 WalkthroughWalkthroughThe PR updates agent HITL interruption handling, adds structured schema-planning models, enforces fully qualified table uniqueness, expands API contracts, and refactors the agent testing page around validated form state and typed resume payloads. ChangesAgent workflow updates
Table identity enforcement
API contracts and agent testing UI
Schema export tooling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AgentTestingPage
participant AgentRouter
participant chat_with_agent
participant agent_graph
AgentTestingPage->>AgentRouter: submit configuration or resume value
AgentRouter->>chat_with_agent: forward tool arguments
chat_with_agent->>agent_graph: inspect saved interrupt state
chat_with_agent->>agent_graph: resume or update state
agent_graph-->>chat_with_agent: final state or interruption
chat_with_agent-->>AgentRouter: return result and interrupt details
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
agent/src/agent/nodes/schema_explorer.py (1)
611-617: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPopulate the
errorfield when structured output parsing fails.The
errorfield was recently added toSchemaExplorerOutput, but it is not being populated in the fallback object when an exception is caught. Including the exception details here prevents the error from being silently swallowed downstream.🐛 Proposed fix
except Exception as e: print(f"Structured output parsing failed in schema explorer: {e}") data = SchemaExplorerOutput( schema_plan=None, ambiguity_detected=False, ambiguity_message="", candidate_options=[], + error=str(e), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/src/agent/nodes/schema_explorer.py` around lines 611 - 617, Update the SchemaExplorerOutput fallback construction in the exception handler to populate its error field with the caught exception details, while preserving the existing default values for schema_plan, ambiguity_detected, ambiguity_message, and candidate_options.backend/app/routers/tables.py (1)
223-224: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winRace condition on concurrent table creation.
There is a Time-of-Check to Time-of-Use (TOCTOU) race condition between the
selectcheck above and thiscommit. If two identical creation requests execute concurrently, both will pass theselectcheck. The second request'scommitwill then violate the new database-levelUniqueConstraintand throw an unhandledIntegrityError, resulting in a 500 Internal Server Error crash instead of the intended 409 Conflict.Catch the
IntegrityErrorto safely fall back to the 409 Conflict response.🔒️ Proposed fix to handle concurrency gracefully
+ from sqlalchemy.exc import IntegrityError + session.add(table) - session.commit() + try: + session.commit() + except IntegrityError: + session.rollback() + raise HTTPException( + status_code=409, + detail=f"Table '{catalog_name}.{schema_name}.{name}' already exists." + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/routers/tables.py` around lines 223 - 224, Update the table-creation flow around session.add(table) and session.commit() to catch the database IntegrityError caused by concurrent duplicate inserts, roll back the session, and return the existing 409 Conflict response instead of propagating a 500 error. Preserve the current pre-insert select check and successful commit behavior.frontend/src/pages/AgentTestingPage.tsx (1)
1372-1381: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
isResumingis alwaysfalsewhen this renders, and the 300ms delay leaves a double-submission window.The parent only mounts this branch when
chatResponse && !chatMutation.isPending(unchanged, line 1373), but is then givenisResuming={chatMutation.isPending}(line 1378) — which is therefore alwaysfalsefor the lifetime of this mount. Combined withhandleApprove/handleReject(lines 1123-1145) callingchatMutation.mutate()inside an un-guardedsetTimeout(..., 300), the Approve/Reject buttons inAgentApprovalForm(lines 590-598,disabled/loading={isResuming}) never actually disable during that 300ms window, so a fast double-click can queue twomutate()calls resuming the samethread_id— a non-idempotent external call to the agent.Tracking submission state locally (set synchronously on click, independent of
chatMutation.isPending) would close this window, e.g.:🔒️ Illustrative fix (needs matching change to the outer render-guard too)
+ const [isSubmittingResume, setIsSubmittingResume] = useState(false); + const handleApprove = (resumeValue?: ResumeValue) => { if (!threadId) return; const config = watch(); + setIsSubmittingResume(true); setTimeout(() => { chatMutation.mutate({ thread_id: threadId, resume_value: resumeValue !== undefined ? resumeValue : { approved: true }, hitl_enabled: config.hitlEnabled, - }); + }, { onSettled: () => setIsSubmittingResume(false) }); }, 300); };Then pass
isResuming={isSubmittingResume}at line 1378 instead ofchatMutation.isPending.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/AgentTestingPage.tsx` around lines 1372 - 1381, Track resume submission locally in the AgentTestingPage approval flow, setting the state synchronously when handleApprove or handleReject is triggered and preventing subsequent submissions until completion. Update the AnimatePresence render guard and the AgentApprovalForm isResuming prop to use this local isSubmittingResume state, so the form remains mounted while submitting and its buttons disable immediately through the delayed chatMutation call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/src/agent/nodes/schema_explorer.py`:
- Around line 172-175: Update the error field definition to use default=None
instead of default_factory=str, preserving its Optional[str] type and existing
description.
In `@backend/app/routers/agent.py`:
- Around line 70-79: Update the TraceSpan construction and its response model
handling so a missing Langfuse observation name/type never produces a None
span_name or response validation failure. Provide a non-null fallback label when
both values are absent, preserving the existing name/type preference and
ensuring all entries returned by the trace endpoint satisfy TraceSpan.span_name:
str.
In `@frontend/openapi.json`:
- Line 1: Confirm that the intentional migration of all endpoints to the /api
prefix has no consumers outside the generated TypeScript client; if external
consumers exist, preserve legacy routes or add a documented deprecation window
before removing them. Track the separate OpenAPI hygiene findings for global
security declarations and maxItems constraints on array schemas, without
expanding this prefix-migration change unless required.
- Line 1: The OpenAPI snapshot and generated frontend types are stale.
Regenerate the spec and frontend types from the current backend models so
`/api/agent/traces/{trace_id}` references the proper `TraceSpan` response schema
and `QueryApproval` contains only `approved` and `feedback`, removing
`rejection_category` and `suggested_fix`.
In `@frontend/src/components/tables/TableList.tsx`:
- Around line 86-91: Update duplicate validation around isDuplicate in TableList
to read the complete fetched table collection from the global query cache rather
than the filtered data variable. Mirror the cache-access pattern used by
OnboardingWizard, then preserve the existing oasis_source_id and composite-name
comparisons against all cached tables.
In `@frontend/src/pages/AgentTestingPage.tsx`:
- Around line 39-64: Replace the hand-written object variant in ResumeValue with
the existing QueryApproval type exported from ../api/agent, while preserving the
string and empty-record variants. Update the import in AgentTestingPage.tsx to
include QueryApproval and derive ResumeValue from that schema type so future
QueryApproval changes remain reflected automatically.
- Around line 1105-1120: The chat mutation setup in the AgentTestingPage
currently applies tableMode only to allowed_statuses while always forwarding
allowedTables. Wire tableMode into the ChatRequest so Exclusive mode excludes
the selected tables, or explicitly rename/reposition the control if it is
intended to govern statuses only; also simplify the redundant allowed_statuses
empty-length expression while preserving the request’s expected empty-array
behavior.
---
Outside diff comments:
In `@agent/src/agent/nodes/schema_explorer.py`:
- Around line 611-617: Update the SchemaExplorerOutput fallback construction in
the exception handler to populate its error field with the caught exception
details, while preserving the existing default values for schema_plan,
ambiguity_detected, ambiguity_message, and candidate_options.
In `@backend/app/routers/tables.py`:
- Around line 223-224: Update the table-creation flow around session.add(table)
and session.commit() to catch the database IntegrityError caused by concurrent
duplicate inserts, roll back the session, and return the existing 409 Conflict
response instead of propagating a 500 error. Preserve the current pre-insert
select check and successful commit behavior.
In `@frontend/src/pages/AgentTestingPage.tsx`:
- Around line 1372-1381: Track resume submission locally in the AgentTestingPage
approval flow, setting the state synchronously when handleApprove or
handleReject is triggered and preventing subsequent submissions until
completion. Update the AnimatePresence render guard and the AgentApprovalForm
isResuming prop to use this local isSubmittingResume state, so the form remains
mounted while submitting and its buttons disable immediately through the delayed
chatMutation call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e384fa67-0f2d-4f66-b8db-0dfa0ee21015
📒 Files selected for processing (16)
agent/src/agent/mcp_server.pyagent/src/agent/nodes/query_builder.pyagent/src/agent/nodes/refiner.pyagent/src/agent/nodes/satisfaction_check.pyagent/src/agent/nodes/schema_explorer.pybackend/alembic/versions/70e8a34ff877_add_unique_constraint_to_tables.pybackend/app/config.pybackend/app/routers/agent.pybackend/app/routers/tables.pycore/src/core/models/models.pyfrontend/openapi.jsonfrontend/src/api/agent.tsfrontend/src/api/schema.d.tsfrontend/src/components/tables/TableList.tsxfrontend/src/components/wizard/OnboardingWizard.tsxfrontend/src/pages/AgentTestingPage.tsx
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
agent/src/agent/nodes/schema_explorer.py (3)
75-82: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCapture the error in the fallback output.
When structured output parsing fails after clarification, the fallback
SchemaExplorerOutputomits theerrorfield. Populate it so the error is tracked consistently with the main parsing logic.🐛 Proposed fix
except Exception as e: logger.error(f"Structured output parsing failed in schema explorer after clarification: {e}") return SchemaExplorerOutput( schema_plan=None, ambiguity_detected=False, ambiguity_message="", candidate_options=[], + error=str(e), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/src/agent/nodes/schema_explorer.py` around lines 75 - 82, Update the exception fallback in the schema explorer’s structured output parsing flow to populate the SchemaExplorerOutput error field with the caught exception, matching the main parsing logic while preserving the existing fallback values.
631-636: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate parsing errors to the agent state.
When structured output parsing fails,
data.erroris populated, but it is not included in the returnedresult_state. As a result, downstream nodes (likequery_builder) will receive an emptyschema_planwith no indication that an error occurred, leading to silent failures or hallucinations.Ensure the error is propagated to the state (e.g., mapped to the existing
last_errorfield).🐛 Proposed fix
result_state: dict = {"schema_plan": plan, "tables_used": tables_used} + if data.error: + result_state["last_error"] = data.error result_state["execution_path"] = ["schema_explorer"] # Store enriched profiles for downstream nodes (refiner re-uses without re-fetch) result_state["table_profiles"] = profile_details if profile_details else None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/src/agent/nodes/schema_explorer.py` around lines 631 - 636, Update the result-state construction in the schema explorer flow to propagate any populated data.error into the existing last_error field. Preserve the current schema_plan, tables_used, execution_path, and table_profiles values while ensuring downstream nodes can detect parsing failures.
627-631: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winAdd
tables_usedtoAgentStateand remove the in-place write.schema_explorer_nodereturnstables_used, andsql_static_validations_nodereads it later, but the shared state schema doesn’t declare the key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent/src/agent/nodes/schema_explorer.py` around lines 627 - 631, Add tables_used to the AgentState schema, then update schema_explorer_node to stop mutating state via state["tables_used"] while retaining tables_used in its returned result_state. Ensure sql_static_validations_node can read the declared shared-state field.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/routers/tables.py`:
- Around line 225-232: Replace the early session.commit() in the table-creation
flow with session.flush() so the table insert, generated table.id, and
constraint validation occur without finalizing the transaction. Preserve the
existing IntegrityError rollback and conflict response, and leave the final
commit after EnrichmentVersion creation as the sole transaction commit.
In `@backend/dump_schema.py`:
- Around line 3-4: Update the output path in the schema-dumping flow around
app.openapi() to resolve openapi.json relative to dump_schema.py’s __file__,
ensuring it targets the repository’s frontend directory regardless of the
process working directory. Preserve the existing JSON serialization and
formatting.
In `@backend/fix_alembic.py`:
- Line 2: Update the create_engine connection setup in fix_alembic.py to obtain
the PostgreSQL URL from an environment variable or existing application
configuration instead of hardcoding credentials. Validate that the configuration
is present and fail clearly when it is missing, while preserving the existing
engine initialization behavior.
- Around line 8-10: Update the Alembic version migration logic around the
conn.execute call to verify that exactly one row with version_num a123e456b789
was updated before committing or printing success. Raise an error on a zero-row
or otherwise unexpected result, and preserve the successful transition to
70e8a34ff877.
In `@frontend/openapi.json`:
- Around line 3531-3541: Remove scoping_mode from frontend/openapi.json and
frontend/src/api/schema.d.ts by regenerating both from
backend/app/routers/agent.py’s ChatRequest, unless ChatRequest is intentionally
updated to accept and forward the field; keep the frontend contract aligned with
the backend model.
In `@frontend/src/components/tables/TableList.tsx`:
- Around line 86-87: Update the duplicate validation around allTables in
TableList to use getQueriesData and aggregate tables from every cached tables
query, rather than reading only the exact unfiltered key or filtered data
fallback. Keep isDuplicate checking the aggregated collection so duplicates
hidden by active filters are still detected.
---
Outside diff comments:
In `@agent/src/agent/nodes/schema_explorer.py`:
- Around line 75-82: Update the exception fallback in the schema explorer’s
structured output parsing flow to populate the SchemaExplorerOutput error field
with the caught exception, matching the main parsing logic while preserving the
existing fallback values.
- Around line 631-636: Update the result-state construction in the schema
explorer flow to propagate any populated data.error into the existing last_error
field. Preserve the current schema_plan, tables_used, execution_path, and
table_profiles values while ensuring downstream nodes can detect parsing
failures.
- Around line 627-631: Add tables_used to the AgentState schema, then update
schema_explorer_node to stop mutating state via state["tables_used"] while
retaining tables_used in its returned result_state. Ensure
sql_static_validations_node can read the declared shared-state field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ccb3b1b-5ed5-45a6-9e45-2ce0e88feced
📒 Files selected for processing (10)
agent/src/agent/nodes/schema_explorer.pyagent/tests/test_scoping_mode.pybackend/app/routers/agent.pybackend/app/routers/tables.pybackend/dump_schema.pybackend/fix_alembic.pyfrontend/openapi.jsonfrontend/src/api/schema.d.tsfrontend/src/components/tables/TableList.tsxfrontend/src/pages/AgentTestingPage.tsx
| try: | ||
| session.commit() | ||
| except IntegrityError: | ||
| session.rollback() | ||
| raise HTTPException( | ||
| status_code=409, | ||
| detail=f"Table '{catalog_name}.{schema_name}.{name}' already exists." | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Ensure atomic creation of the table and its enrichment data.
Using session.commit() here commits the table to the database before the associated EnrichmentVersion is created. If the subsequent commit on line 257 fails, it leaves the database in an inconsistent state (a table without its enrichment data).
Using session.flush() executes the insert, populates table.id, and validates constraints (raising IntegrityError if necessary), but defers the transaction commit until the enrichment data is also added, making the entire operation atomic.
♻️ Proposed fix to ensure atomicity
try:
- session.commit()
+ session.flush()
except IntegrityError:
session.rollback()
raise HTTPException(
status_code=409,
detail=f"Table '{catalog_name}.{schema_name}.{name}' already exists."
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| session.commit() | |
| except IntegrityError: | |
| session.rollback() | |
| raise HTTPException( | |
| status_code=409, | |
| detail=f"Table '{catalog_name}.{schema_name}.{name}' already exists." | |
| ) | |
| try: | |
| session.flush() | |
| except IntegrityError: | |
| session.rollback() | |
| raise HTTPException( | |
| status_code=409, | |
| detail=f"Table '{catalog_name}.{schema_name}.{name}' already exists." | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/routers/tables.py` around lines 225 - 232, Replace the early
session.commit() in the table-creation flow with session.flush() so the table
insert, generated table.id, and constraint validation occur without finalizing
the transaction. Preserve the existing IntegrityError rollback and conflict
response, and leave the final commit after EnrichmentVersion creation as the
sole transaction commit.
| with open("../frontend/openapi.json", "w") as f: | ||
| json.dump(app.openapi(), f, indent=2) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- backend/dump_schema.py ---\n'
cat -n backend/dump_schema.py
printf '\n--- frontend/package.json ---\n'
cat -n frontend/package.json
printf '\n--- path resolution probe ---\n'
python3 - <<'PY'
from pathlib import Path
cwd = Path.cwd()
script_dir = Path('backend').resolve()
print('cwd:', cwd)
print('script_dir:', script_dir)
print('open("../frontend/openapi.json") from repo root ->', (cwd / '../frontend/openapi.json').resolve())
print('expected repo frontend path ->', (cwd / 'frontend/openapi.json').resolve())
print('from backend dir ->', (script_dir / '../frontend/openapi.json').resolve())
PYRepository: StavPonte11/text2sql-onboarding
Length of output: 260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- backend/dump_schema.py ---'
cat -n backend/dump_schema.py
echo
echo '--- frontend/package.json ---'
cat -n frontend/package.json
echo
echo '--- repo path probe ---'
python3 - <<'PY'
from pathlib import Path
cwd = Path.cwd()
print('cwd:', cwd)
print('repo frontend path:', (cwd / 'frontend/openapi.json').resolve())
print('relative "../frontend/openapi.json" from repo root:', (cwd / '../frontend/openapi.json').resolve())
PYRepository: StavPonte11/text2sql-onboarding
Length of output: 3440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- references to dump_schema.py ---'
rg -n "dump_schema\.py|openapi\.json" -g '!**/node_modules/**' -g '!**/.git/**' .
echo
echo '--- backend tree ---'
git ls-files backend frontend | sed -n '1,200p'Repository: StavPonte11/text2sql-onboarding
Length of output: 6192
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- references to backend/dump_schema.py in docs and scripts ---'
rg -n "dump_schema\.py|openapi\.json|generate-types|schema\.json|schema\.d.ts" README.md backend/README.md frontend/README.md backend frontend -g '!**/node_modules/**' -g '!**/.git/**'
echo
echo '--- backend README (if present) ---'
if [ -f backend/README.md ]; then
cat -n backend/README.md
fi
echo
echo '--- top-level README (if present) ---'
if [ -f README.md ]; then
cat -n README.md
fiRepository: StavPonte11/text2sql-onboarding
Length of output: 570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- backend/Dockerfile ---'
cat -n backend/Dockerfile
echo
echo '--- backend/pyproject.toml ---'
cat -n backend/pyproject.toml
echo
echo '--- backend/e2e_test.py (if it mentions schema generation) ---'
rg -n "dump_schema|openapi.json|frontend/openapi.json|schema.d.ts" backend -g '!**/.git/**'Repository: StavPonte11/text2sql-onboarding
Length of output: 4975
Anchor openapi.json to __file__
open("../frontend/openapi.json", "w") only works when the process starts in backend/; from the repo root it resolves outside the repo. Use a script-relative path here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/dump_schema.py` around lines 3 - 4, Update the output path in the
schema-dumping flow around app.openapi() to resolve openapi.json relative to
dump_schema.py’s __file__, ensuring it targets the repository’s frontend
directory regardless of the process working directory. Preserve the existing
JSON serialization and formatting.
| @@ -0,0 +1,10 @@ | |||
| from sqlalchemy import create_engine, text | |||
| engine = create_engine('postgresql://postgres:postgres@localhost:5432/text2sql') | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not commit database credentials in the script.
The PostgreSQL username, password, host, port, and database are exposed in source control. Read the connection URL from an environment variable or existing application configuration, and fail if it is missing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/fix_alembic.py` at line 2, Update the create_engine connection setup
in fix_alembic.py to obtain the PostgreSQL URL from an environment variable or
existing application configuration instead of hardcoding credentials. Validate
that the configuration is present and fail clearly when it is missing, while
preserving the existing engine initialization behavior.
| conn.execute(text("UPDATE alembic_version SET version_num = '70e8a34ff877' WHERE version_num = 'a123e456b789'")) | ||
| conn.commit() | ||
| print("Updated to 70e8a34ff877") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail when the expected Alembic revision was not updated.
If alembic_version.version_num is not exactly a123e456b789, this update affects zero rows, but the script still prints Updated to 70e8a34ff877. Check the update result or re-query the table and raise an error on a no-op; otherwise operators may believe the database is repaired when it is not.
The target revision is confirmed by backend/alembic/versions/70e8a34ff877_add_unique_constraint_to_tables.py.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 10-10: print found
Remove print
(T201)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/fix_alembic.py` around lines 8 - 10, Update the Alembic version
migration logic around the conn.execute call to verify that exactly one row with
version_num a123e456b789 was updated before committing or printing success.
Raise an error on a zero-row or otherwise unexpected result, and preserve the
successful transition to 70e8a34ff877.
| "scoping_mode": { | ||
| "anyOf": [ | ||
| { | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "type": "null" | ||
| } | ||
| ], | ||
| "title": "Scoping Mode" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Backend ChatRequest and scoping_mode occurrences:\n'
git ls-files | rg 'backend/app/routers/agent\.py|frontend/openapi\.json|frontend/src/api/schema\.d\.ts|package\.json|pyproject\.toml|Makefile|README\.md|openapi'
printf '\n--- backend/app/routers/agent.py (relevant lines) ---\n'
sed -n '1,140p' backend/app/routers/agent.py
printf '\n--- search scoping_mode ---\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'scoping_mode' backend frontend .github . || trueRepository: StavPonte11/text2sql-onboarding
Length of output: 7791
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' backend/app/routers/agent.pyRepository: StavPonte11/text2sql-onboarding
Length of output: 3864
scoping_mode should not be in the frontend contract unless the backend proxy accepts it. backend/app/routers/agent.py’s ChatRequest has no scoping_mode, so the field in frontend/openapi.json and frontend/src/api/schema.d.ts is stale and will be dropped at the API boundary. Regenerate the spec/types from the backend model, or add the field to ChatRequest if it’s meant to be forwarded.
📍 Affects 2 files
frontend/openapi.json#L3531-L3541(this comment)frontend/src/api/schema.d.ts#L1332-L1333
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/openapi.json` around lines 3531 - 3541, Remove scoping_mode from
frontend/openapi.json and frontend/src/api/schema.d.ts by regenerating both from
backend/app/routers/agent.py’s ChatRequest, unless ChatRequest is intentionally
updated to accept and forward the field; keep the frontend contract aligned with
the backend model.
| const allTables = qc.getQueryData<Table[]>(['tables', '', '']) || data || []; | ||
| const isDuplicate = allTables.some( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Client-side duplicate validation is bypassed when filters are active.
Using getQueryData with an exact ['tables', '', ''] key returns undefined if the user hasn't explicitly fetched the unfiltered list (e.g., if a filter was active on page load). It then falls back to the currently filtered data, meaning the duplicate check will incorrectly pass for duplicates hidden by the active filter.
Use getQueriesData to aggregate all cached table lists, mirroring the previous recommendation.
🐛 Proposed fix
- const allTables = qc.getQueryData<Table[]>(['tables', '', '']) || data || [];
- const isDuplicate = allTables.some(
+ const queriesData = qc.getQueriesData<Table[]>({ queryKey: ['tables'] });
+ const existingTables = queriesData.flatMap(([_, d]) => d || []);
+ const isDuplicate = existingTables.some(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/tables/TableList.tsx` around lines 86 - 87, Update
the duplicate validation around allTables in TableList to use getQueriesData and
aggregate tables from every cached tables query, rather than reading only the
exact unfiltered key or filtered data fallback. Keep isDuplicate checking the
aggregated collection so duplicates hidden by active filters are still detected.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
API Updates
/api/...route naming.Tests & Migrations