Skip to content

Execution results APP-7350#7

Merged
ivpusic merged 2 commits into
query_runfrom
executions
Mar 3, 2026
Merged

Execution results APP-7350#7
ivpusic merged 2 commits into
query_runfrom
executions

Conversation

@ivpusic
Copy link
Copy Markdown
Member

@ivpusic ivpusic commented Mar 2, 2026

dune execution results 01JG2A3B4C5D6E7F8G9H0J1K2M

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 2, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Member Author

ivpusic commented Mar 2, 2026

@ivpusic ivpusic changed the title task complete Execution results Mar 3, 2026
@ivpusic ivpusic changed the title Execution results Execution results APP-7350 Mar 3, 2026
@linear
Copy link
Copy Markdown

linear Bot commented Mar 3, 2026

APP-7350 Step 9: `dune query results`

cmd/query/results.go — positional arg: execution ID (string). Flags: --limit, --offset, -o.

One-shot fetch via QueryResultsV2(executionID, options) — no polling. If still running: print status, exit 0. If complete: display results. If failed: print error, exit 1.

Acceptance criteria:

  • Completed execution displays results
  • --limit and --offset work
  • Running execution prints status, exits 0
  • Failed execution prints error, exits 1
  • -o json and -o csv work

Tests:

  • Completed execution renders results (mock API)
  • Running execution prints status
  • Failed execution prints error
  • Offset and limit passed correctly
  • Missing argument errors

@ivpusic ivpusic requested a review from va3093 March 3, 2026 08:47
@ivpusic ivpusic marked this pull request as ready for review March 3, 2026 08:47
@cursor
Copy link
Copy Markdown

cursor Bot commented Mar 3, 2026

PR Summary

Medium Risk
Adds a new top-level CLI command that calls the executions results API and introduces new branching output/error handling for execution states. Risk is moderate due to new user-facing command behavior and pagination options, but changes are isolated to CLI/output code.

Overview
Adds a new dune execution results <execution-id> command (registered on the root command) to fetch execution results via QueryResultsV2, supporting --limit/--offset pagination and -o json output.

Refactors result rendering used by dune query run into shared output.DisplayResults/output.ResultRowsToStrings, and adds comprehensive tests covering completed/pending/executing/failed/cancelled states plus pagination option wiring.

Written by Cursor Bugbot for commit c9274b4. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Negative flag value wraps to huge unsigned number
    • Added individual non-negative validation for limit and offset flags before casting to unsigned types to prevent integer wrapping.

Create PR

Or push these changes by commenting:

@cursor push db5f18399a
Preview (db5f18399a)
diff --git a/cmd/execution/results.go b/cmd/execution/results.go
--- a/cmd/execution/results.go
+++ b/cmd/execution/results.go
@@ -30,6 +30,13 @@
 	limit, _ := cmd.Flags().GetInt("limit")
 	offset, _ := cmd.Flags().GetInt("offset")
 
+	if limit < 0 {
+		return fmt.Errorf("limit must be non-negative, got %d", limit)
+	}
+	if offset < 0 {
+		return fmt.Errorf("offset must be non-negative, got %d", offset)
+	}
+
 	opts := models.ResultOptions{}
 	if limit > 0 || offset > 0 {
 		opts.Page = &models.ResultPageOption{

Comment @cursor review or bugbot run to trigger another review on this PR

Comment thread cmd/execution/results.go
Comment thread cmd/execution/results.go
case "QUERY_STATE_CANCELLED":
return fmt.Errorf("execution was cancelled")
default:
return fmt.Errorf("unexpected execution state: %s", resp.State)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Known QUERY_STATE_EXPIRED state treated as unexpected

Low Severity

The plan document explicitly lists QUERY_STATE_EXPIRED as a valid API state, but the switch statement has no case for it. It falls through to the default branch, producing the misleading message "unexpected execution state: QUERY_STATE_EXPIRED" — even though it's a known, expected state that deserves a clearer user-facing message.

Fix in Cursor Fix in Web

@ivpusic
Copy link
Copy Markdown
Member Author

ivpusic commented Mar 3, 2026

@cursor push db5f183

Add validation to ensure limit and offset flags are non-negative before casting to unsigned types. This prevents negative values from wrapping to large positive numbers (e.g., uint64(-1) = 18446744073709551615).

Applied via @cursor push command
@ivpusic ivpusic merged commit 222425a into query_run Mar 3, 2026
1 check passed
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.

3 participants