feat:nao test: assert the agent performed a specific action - #1408
feat:nao test: assert the agent performed a specific action#1408justin212407 wants to merge 3 commits into
Conversation
Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
|
This PR was auto-closed. Only contributors approved with Maintainers review auto-closed issues daily. Issues that do not meet the quality bar in CONTRIBUTING.md will not be reopened or receive a reply. If a maintainer replies See CONTRIBUTING.md. |
|
@Bl3f ptal and lmk if there are any changes that you might want me to make |
socallmebertille
left a comment
There was a problem hiding this comment.
Hi, thanks for your contribution!
You should remove the !testMode condition into apps/backend/src/components/ai/system-prompt.tsx (l. 109) if you want to make available the clarification tool in test mode.
| // Keep clarification available in testMode so `nao test` can assert follow-up | ||
| // questions and other intermediate actions on the recorded tool-call trace. |
There was a problem hiding this comment.
Maybe we should avoid cluttering this file with this comment, this is basically information you've already included in the PR description, there is no need to repeat it here again
| // Keep clarification available in testMode so `nao test` can assert follow-up | |
| // questions and other intermediate actions on the recorded tool-call trace. |
| /** | ||
| * @deprecated No longer strips tools. Kept so existing callers that pass | ||
| * `testMode` continue to typecheck. Clarification stays available so | ||
| * `nao test` can assert intermediate actions (e.g. follow-up questions). | ||
| */ | ||
| testMode?: boolean; |
There was a problem hiding this comment.
You can delete this part as long as you remove the three call-site props in the same edit.
| /** | |
| * @deprecated No longer strips tools. Kept so existing callers that pass | |
| * `testMode` continue to typecheck. Clarification stays available so | |
| * `nao test` can assert intermediate actions (e.g. follow-up questions). | |
| */ | |
| testMode?: boolean; |
There was a problem hiding this comment.
I think this file is unnecessary
Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
|
I have removed the |
There was a problem hiding this comment.
All reported issues were addressed across 14 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cli/nao_core/config/databases/duckdb.py">
<violation number="1" location="cli/nao_core/config/databases/duckdb.py:24">
P2: When a DuckDB schema or table name contains an apostrophe, this SQL becomes invalid, so `description()` and `_fetch_column_descriptions()` silently omit comments. Escape these values as SQL string literals or bind them before constructing both metadata queries.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| try: | ||
| query = ( | ||
| "SELECT comment FROM duckdb_tables() " | ||
| f"WHERE schema_name = '{self._schema}' AND table_name = '{self._table_name}'" |
There was a problem hiding this comment.
P2: When a DuckDB schema or table name contains an apostrophe, this SQL becomes invalid, so description() and _fetch_column_descriptions() silently omit comments. Escape these values as SQL string literals or bind them before constructing both metadata queries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/nao_core/config/databases/duckdb.py, line 24:
<comment>When a DuckDB schema or table name contains an apostrophe, this SQL becomes invalid, so `description()` and `_fetch_column_descriptions()` silently omit comments. Escape these values as SQL string literals or bind them before constructing both metadata queries.</comment>
<file context>
@@ -15,6 +15,42 @@
+ try:
+ query = (
+ "SELECT comment FROM duckdb_tables() "
+ f"WHERE schema_name = '{self._schema}' AND table_name = '{self._table_name}'"
+ )
+ row = self._fetchone(self._conn.raw_sql(query)) # type: ignore[union-attr]
</file context>
Description
Today,
nao testcan only verify final agent output by comparing the generated SQL's result against a referencesqldataframe. There's no way to assert on intermediate agent behavior, so a test case can't check things like "did the agent ask a clarifying question before running SQL" or "did the agent call a specific tool."This adds an extensible
assertionsmechanism tonao testthat checks the agent's recorded tool-call trace independently of (and combinable with) the existing dataframe-based SQL verification.Changes made
Assertionframework (cli/nao_core/commands/test/assertions.py):introduces a
ToolCallAssertiontype that checks whether a named tool was invoked, with optionalargssubset-matching (recursive for nested dicts) andmin_count.parse_assertions()validates theassertionslist from a test YAML and raisesAssertionConfigErroron malformed entries (missingtype, unknown fields, invalidmin_count, etc.).evaluate_assertions()runs all assertions against the tool-call trace and returns a combined pass/fail with a human-readable message.TestCase(cli/nao_core/commands/test/case.py) now carries anassertions: list[Assertion]field, parsed from the test YAML alongside the existingsqlfield, which is now optional (sql: str | None = None) so assertion-only tests don't need reference SQL.runner.py: assertions are evaluated alongside SQL verification when both are present (passed = assertions_passed and data_passed), and a new code path handles assertion-only test cases (nosql) by reporting pass/fail on the assertions alone.apps/backend/src/routes/test.tsmakes thesqlfield in the test request body optional (defaults to''), andapps/backend/src/agents/tools/index.tskeeps theclarificationtool available intestMode(previously stripped) sonao testcan assert on clarifying follow-up questions and other intermediate actions in the recorded tool-call trace.cli/README.mddocuments the newassertionsYAML syntax with atool_callexample (asserting aclarificationfollow-up), and explains thattool_callassertions supporttool,args, andmin_count, and that SQL verification and assertions can be combined.Screenshots
Example test case
Validation
tests/nao_core/commands/test_assertions.py,test_case.py,test_runner.pycovering assertion parsing/validation,TestCaseYAML loading with assertions, and runner behavior for combined SQL + assertion checks and assertion-only test cases.apps/backend/src/routes/test.ts— verified optionalsqlfield doesn't break existing SQL-based test payloads.sql) runs end-to-end throughnao testand reports pass/fail based solely on the tool-call trace.