Make --continue/-c govern every input shape in validate - #846
Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
🤖 Augment PR SummarySummary: This PR makes default validation stop after the first failing instance across more input shapes. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| if (!suboutput.at("valid").to_boolean()) { | ||
| result = false; | ||
| if (entry.multidocument && !continue_on_error) { | ||
| if (!continue_on_error) { |
There was a problem hiding this comment.
In --benchmark mode, process_entry returns at line 174 before reaching this check, so a failing entry in a supported JSONL input is followed by every remaining entry even when --continue is absent. This contradicts the new stop-at-first-failure contract for validate.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
3 issues found across 7 files
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="test/validate/fail_directory_continue_verbose.clitest">
<violation number="1" location="test/validate/fail_directory_continue_verbose.clitest:26">
P3: This failure test validates directory input with `--continue`, but only in human-readable text mode. The team convention for failure cases is to also assert the structured `--json` output; both `fail_directory_json.clitest` and `fail_directory_fast_json.clitest` follow that pattern. Add a JSON variant (e.g. `fail_directory_continue_json_verbose.clitest`) invoking `validate schema.json instances --json --continue --verbose` so error reporting is verified consistently across both output formats.</violation>
</file>
<file name="test/validate/fail_directory_stop_verbose.clitest">
<violation number="1" location="test/validate/fail_directory_stop_verbose.clitest:26">
P3: This `fail_directory_stop_verbose` test asserts the text/stderr failure output for the directory stop-on-error behavior, but it has no `--json` companion. The team convention for `fail_` cases is to add a JSON variant that asserts the structured error (e.g., `fail_directory_json.clitest`, `fail_directory_fast_json.clitest`, and the `fail_jsonl_*_continue_json*` set all do this). Add a `fail_directory_stop_json` test that runs `validate schema.json instances --json` and asserts the JSON error object, so the stop-on-error behavior is also covered for JSON-consuming tools.</violation>
</file>
<file name="test/validate/fail_many_continue_verbose.clitest">
<violation number="1" location="test/validate/fail_many_continue_verbose.clitest:28">
P3: This new failure test only asserts the human-readable (text) output, via `--verbose`, for the new `--continue` behavior over multiple file inputs. For error paths the team expects an equivalent `--json` variant that asserts the structured error, so `--continue` is verified in both the text and machine-readable output formats. Add e.g. a `fail_many_continue_json_verbose.clitest` (or a json run in this file) that runs `validate … --continue --json` and asserts the JSON error output.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| EOF | ||
|
|
||
| // Validation failure | ||
| RUN validate schema.json instances --continue --verbose STDIN /dev/null IN . INTO result_0.txt EXPECTING 2 |
There was a problem hiding this comment.
P3: This failure test validates directory input with --continue, but only in human-readable text mode. The team convention for failure cases is to also assert the structured --json output; both fail_directory_json.clitest and fail_directory_fast_json.clitest follow that pattern. Add a JSON variant (e.g. fail_directory_continue_json_verbose.clitest) invoking validate schema.json instances --json --continue --verbose so error reporting is verified consistently across both output formats.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/validate/fail_directory_continue_verbose.clitest, line 26:
<comment>This failure test validates directory input with `--continue`, but only in human-readable text mode. The team convention for failure cases is to also assert the structured `--json` output; both `fail_directory_json.clitest` and `fail_directory_fast_json.clitest` follow that pattern. Add a JSON variant (e.g. `fail_directory_continue_json_verbose.clitest`) invoking `validate schema.json instances --json --continue --verbose` so error reporting is verified consistently across both output formats.</comment>
<file context>
@@ -0,0 +1,43 @@
+EOF
+
+// Validation failure
+RUN validate schema.json instances --continue --verbose STDIN /dev/null IN . INTO result_0.txt EXPECTING 2
+
+REPLACE $CWD WITH '[CWD]' IN result_0.txt
</file context>
| EOF | ||
|
|
||
| // Validation failure | ||
| RUN validate schema.json instance_1.json instance_2.json instance_3.json --continue --verbose STDIN /dev/null IN . INTO result_0.txt EXPECTING 2 |
There was a problem hiding this comment.
P3: This new failure test only asserts the human-readable (text) output, via --verbose, for the new --continue behavior over multiple file inputs. For error paths the team expects an equivalent --json variant that asserts the structured error, so --continue is verified in both the text and machine-readable output formats. Add e.g. a fail_many_continue_json_verbose.clitest (or a json run in this file) that runs validate … --continue --json and asserts the JSON error output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/validate/fail_many_continue_verbose.clitest, line 28:
<comment>This new failure test only asserts the human-readable (text) output, via `--verbose`, for the new `--continue` behavior over multiple file inputs. For error paths the team expects an equivalent `--json` variant that asserts the structured error, so `--continue` is verified in both the text and machine-readable output formats. Add e.g. a `fail_many_continue_json_verbose.clitest` (or a json run in this file) that runs `validate … --continue --json` and asserts the JSON error output.</comment>
<file context>
@@ -0,0 +1,47 @@
+EOF
+
+// Validation failure
+RUN validate schema.json instance_1.json instance_2.json instance_3.json --continue --verbose STDIN /dev/null IN . INTO result_0.txt EXPECTING 2
+
+REPLACE $CWD WITH '[CWD]' IN result_0.txt
</file context>
| EOF | ||
|
|
||
| // Validation failure | ||
| RUN validate schema.json instances --verbose STDIN /dev/null IN . INTO result_0.txt EXPECTING 2 |
There was a problem hiding this comment.
P3: This fail_directory_stop_verbose test asserts the text/stderr failure output for the directory stop-on-error behavior, but it has no --json companion. The team convention for fail_ cases is to add a JSON variant that asserts the structured error (e.g., fail_directory_json.clitest, fail_directory_fast_json.clitest, and the fail_jsonl_*_continue_json* set all do this). Add a fail_directory_stop_json test that runs validate schema.json instances --json and asserts the JSON error object, so the stop-on-error behavior is also covered for JSON-consuming tools.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/validate/fail_directory_stop_verbose.clitest, line 26:
<comment>This `fail_directory_stop_verbose` test asserts the text/stderr failure output for the directory stop-on-error behavior, but it has no `--json` companion. The team convention for `fail_` cases is to add a JSON variant that asserts the structured error (e.g., `fail_directory_json.clitest`, `fail_directory_fast_json.clitest`, and the `fail_jsonl_*_continue_json*` set all do this). Add a `fail_directory_stop_json` test that runs `validate schema.json instances --json` and asserts the JSON error object, so the stop-on-error behavior is also covered for JSON-consuming tools.</comment>
<file context>
@@ -0,0 +1,41 @@
+EOF
+
+// Validation failure
+RUN validate schema.json instances --verbose STDIN /dev/null IN . INTO result_0.txt EXPECTING 2
+
+REPLACE $CWD WITH '[CWD]' IN result_0.txt
</file context>
Signed-off-by: Juan Cruz Viotti jv@jviotti.com