feat: validate format param, surface in tool prose, add opt-in JSON unwrap (#58)

* feat: validate format parameter, surface in tool prose, add opt-in JSON unwrap

- Reject invalid format values with a clear error naming accepted values
  (json, csv, markdown) instead of silently falling through to JSON default
- Add format and unwrap_json documentation to trino_query and trino_execute
  tool description prose so MCP agents discover them without reading param schemas
- Add opt-in unwrap_json parameter that parses single-row, single-VARCHAR-column
  JSON results into an unwrapped_result field, reducing double-decode overhead
  for table functions like raw_query
- Extract shared formatOutput/validateFormat/tryUnwrapJSON helpers to format.go

Closes #57

* fix: address all review findings from PR #58

Bug fixes:
- unwrap_json now renders the QueryOutput (with unwrapped_result) into
  the TextContent so the LLM actually sees clean JSON in its context,
  not just in the typed structured output
- formatOutput uses explicit case "json" instead of default, with an
  error-returning default for unsupported formats

Design fixes:
- tryUnwrapJSON rejects bare JSON scalars (strings, numbers, booleans,
  null) — only objects and arrays are meaningful to unwrap
- Accept CHAR, CHAR(N), and JSON column types in addition to VARCHAR
  for unwrap detection via new isStringColumnType helper
- Validate trino_explain type parameter with same pattern as format:
  invalid values return a clear error instead of silently defaulting

Code quality:
- Extract renderTextContent helper to reduce cyclomatic complexity in
  handleQuery (18→14) and handleExecute (17→13)
- Extract output format string constants to satisfy goconst linter
- Move nolint:lll to end of same line (conventional placement)
- Revert unnecessary struct field alignment in QueryOutput
- Simplify JSON comparison in tests (direct string comparison)

Test additions:
- unwrap_json with CSV format: structured output has unwrapped_result,
  CSV text does not
- Scalar JSON values ("hello", 42, true, null) are not unwrapped
- Invalid explain type returns error
- Text content assertions for unwrap_json (verifies issue #1 fix)

* fix: unwrap JSON inline in rows instead of sidecar field (Option 2)

Redesign unwrap_json to mutate the QueryOutput in place rather than
adding a separate unwrapped_result field. When the flag is on and the
result matches (1 row, 1 string-type column, valid JSON object/array):

- columns[0].type is changed to "JSON" as an unambiguous signal
- rows[0][col] is replaced with the parsed object
- the envelope shape is identical to the non-unwrap case

This eliminates the double-payload problem (old design serialized both
the escaped string in rows AND the parsed object in unwrapped_result),
the schema divergence (old design rendered QueryOutput vs QueryResult
depending on the flag), and delivers the actual token-cost reduction
that motivated the feature.

Specific changes:
- Remove UnwrappedResult from QueryOutput and renderTextContent helper
- Add unwrapJSONColumn that mutates QueryOutput in place
- Move formatCSV/formatMarkdown to format.go, change to accept *QueryOutput
- Add stringifyValue helper so CSV/markdown render maps as compact JSON
- Add columnTypeJSON constant for the "JSON" type signal
- Update descriptions to say "single-string-column" (matches CHAR/JSON too)
- Rewrite all unwrap tests to assert column type and row value, not sidecar

* fix: description mismatch, shared slice mutation, code organization

- Fix ToolExecute description to match ToolQuery: "single-string-column"
  with full column-type-changes-to-JSON language (was still saying
  "single-VARCHAR-column" from the first commit)
- Shallow-copy the row map in unwrapJSONColumn before mutation to avoid
  modifying the shared client.QueryResult.Rows slice
- Move escapeCSV from query.go to format.go alongside its only caller
- Remove dead explain_test.go cases for inputs now rejected by validation

Author: cjimti

pkg/tools/descriptions.go

@@ -9,12 +9,22 @@ var defaultDescriptions = map[ToolName]string{
 		"excessive data transfer — use the limit parameter to control result size. " +
 		"For large tables, always include WHERE clauses to filter results. " +
 		"Consider using trino_explain first for expensive queries. " +
+		"Results are returned as JSON by default. Pass format=csv for CSV output " +
+		"(more token-efficient for large result sets) or format=markdown for a pipe-table. " +
+		"Set unwrap_json=true to automatically parse single-row, single-string-column " +
+		"results containing a JSON object or array — the column type changes to JSON " +
+		"and the value becomes the parsed object (common with table functions like raw_query). " +
 		"For write operations (INSERT, CREATE, etc.), use trino_execute instead.",
 
 	ToolExecute: "Execute a SQL statement against Trino, including write operations " +
 		"(INSERT, UPDATE, DELETE, CREATE, DROP, ALTER, etc.). Returns affected row " +
 		"counts or query results. Use trino_query for read-only SELECT queries. " +
-		"This tool should be used when you need to modify data or schema.",
+		"This tool should be used when you need to modify data or schema. " +
+		"Results are returned as JSON by default. Pass format=csv for CSV output " +
+		"(more token-efficient for large result sets) or format=markdown for a pipe-table. " +
+		"Set unwrap_json=true to automatically parse single-row, single-string-column " +
+		"results containing a JSON object or array — the column type changes to JSON " +
+		"and the value becomes the parsed object (common with table functions like raw_query).",
 
 	ToolExplain: "Get the execution plan for a SQL query to understand performance characteristics " +
 		"before running expensive queries. Use this when querying large tables (millions of " +

pkg/tools/execute.go

@@ -2,7 +2,6 @@ package tools
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"time"
 
@@ -25,6 +24,11 @@ type ExecuteInput struct {
 	// Format is the output format: json (default), csv, or markdown.
 	Format string `json:"format,omitempty" jsonschema_description:"Output format: json, csv, or markdown (default: json)"`
 
+	// UnwrapJSON controls automatic JSON unwrapping for single-row, single-string-column results.
+	// When true and the result matches, the column type is changed to "JSON" and the row value
+	// is replaced with the parsed object. The envelope shape is unchanged.
+	UnwrapJSON bool `json:"unwrap_json,omitempty" jsonschema_description:"When true and result is a single-row single-string-column containing a JSON object or array, parse the value inline and set column type to JSON"` //nolint:lll // jsonschema_description must be a single tag value
+
 	// Connection is the named connection to use. Empty uses the default connection.
 	// Use trino_list_connections to see available connections.
 	Connection string `json:"connection,omitempty" jsonschema_description:"Named connection to use (see trino_list_connections)"`
@@ -68,6 +72,11 @@ func (t *Toolkit) handleExecute(ctx context.Context, _ *mcp.CallToolRequest, inp
 		return ErrorResult("sql parameter is required"), nil, nil
 	}
 
+	// Validate format
+	if err := validateFormat(input.Format); err != nil {
+		return ErrorResult(err.Error()), nil, nil
+	}
+
 	// Apply query interceptors (no read-only enforcement — that's the point of trino_execute)
 	sql, err := t.InterceptSQL(ctx, input.SQL, ToolExecute)
 	if err != nil {
@@ -117,32 +126,24 @@ func (t *Toolkit) handleExecute(ctx context.Context, _ *mcp.CallToolRequest, inp
 	notifyProgress(ctx, notifier, 1, 3,
 		fmt.Sprintf("Statement returned %d rows, formatting...", result.Stats.RowCount))
 
-	// Format output
-	format := input.Format
-	if format == "" {
-		format = "json"
+	// Build structured output (reuse QueryOutput — same result shape)
+	queryOutput := buildQueryOutput(result)
+
+	// Attempt JSON unwrap if requested — mutates queryOutput in place,
+	// changing columns[0].type to "JSON" and replacing the row value.
+	if input.UnwrapJSON {
+		unwrapJSONColumn(&queryOutput)
 	}
 
-	var output string
-	switch format {
-	case "csv":
-		output = formatCSV(result)
-	case "markdown":
-		output = formatMarkdown(result)
-	default:
-		data, err := json.MarshalIndent(result, "", "  ")
-		if err != nil {
-			return ErrorResult(fmt.Sprintf("Failed to marshal result: %v", err)), nil, nil
-		}
-		output = string(data)
+	// Format output
+	output, err := formatOutput(&queryOutput, input.Format)
+	if err != nil {
+		return ErrorResult(err.Error()), nil, nil
 	}
 
 	// Send progress notification: complete
 	notifyProgress(ctx, notifier, 2, 3, "Execution complete")
 
-	// Build structured output (reuse QueryOutput — same result shape)
-	queryOutput := buildQueryOutput(result)
-
 	return &mcp.CallToolResult{
 		Content: []mcp.Content{
 			&mcp.TextContent{Text: output},

pkg/tools/explain.go

@@ -59,6 +59,11 @@ func (t *Toolkit) handleExplain(ctx context.Context, _ *mcp.CallToolRequest, inp
 		return ErrorResult("sql parameter is required"), nil, nil
 	}
 
+	// Validate explain type
+	if err := validateExplainType(input.Type); err != nil {
+		return ErrorResult(err.Error()), nil, nil
+	}
+
 	// Apply query interceptors
 	sql, err := t.InterceptSQL(ctx, input.SQL, ToolExplain)
 	if err != nil {

pkg/tools/explain_test.go

@@ -79,16 +79,9 @@ func TestExplainInput_TypeMapping(t *testing.T) {
 			inputType:    "",
 			expectedType: client.ExplainLogical,
 		},
-		{
-			name:         "unknown defaults to logical",
-			inputType:    "unknown",
-			expectedType: client.ExplainLogical,
-		},
-		{
-			name:         "LOGICAL uppercase defaults to logical",
-			inputType:    "LOGICAL",
-			expectedType: client.ExplainLogical, // Case sensitive, defaults
-		},
+		// "unknown" and "LOGICAL" are now rejected by validateExplainType
+		// before reaching the switch. Invalid types are tested in
+		// TestHandleExplain_InvalidType in handlers_test.go.
 	}
 
 	for _, tt := range tests {