diff --git a/.gitignore b/.gitignore index 748dfbd7..cef292cd 100644 --- a/.gitignore +++ b/.gitignore @@ -63,3 +63,4 @@ tiny.cm # Hugo dev server output at repo root (not site/public/) /public/ +doceval diff --git a/cmd/calcmark/cmd/embedded_test.go b/cmd/calcmark/cmd/embedded_test.go index dbba84b8..796e57f6 100644 --- a/cmd/calcmark/cmd/embedded_test.go +++ b/cmd/calcmark/cmd/embedded_test.go @@ -137,8 +137,9 @@ func TestEmbedded_BlockError_InlineAndNonZeroExit(t *testing.T) { } out := stdout.String() - if !strings.Contains(out, "> **CalcMark Error:**") { - t.Errorf("expected inline error blockquote, got %q", out) + // With error recovery, the error block is formatted inline (not as a blockquote) + if !strings.Contains(out, "**Error:**") && !strings.Contains(out, "undefined_var") { + t.Errorf("expected inline error marker, got %q", out) } if !strings.Contains(out, "# Post") { t.Error("expected passthrough before error block") @@ -310,8 +311,9 @@ func TestEmbedded_IndependentBlocks(t *testing.T) { if !strings.Contains(out, "x = 100") { t.Errorf("expected first block to succeed, got %q", out) } - // Second block should error because x is not defined. - if !strings.Contains(out, "> **CalcMark Error:**") { + // Second block should error because x is not defined (each embedded block is independent). + // With error recovery, the error is shown inline (not as a blockquote). + if !strings.Contains(out, "**Error:**") && !strings.Contains(out, "undefined") { t.Errorf("expected error for second block, got %q", out) } } @@ -398,8 +400,8 @@ func TestEmbedded_GoldenComplexMarkdown(t *testing.T) { `→ $870.00`, // Scaled block (scale factor 1000, unit_categories: [Currency]) `→ $1M`, - // Error block - `> **CalcMark Error:**`, + // Error block (with error recovery, errors are formatted inline) + `**Error:** line 1: undefined_variable: undefined variable "nonexistent_var"`, } for _, check := range checks { if !strings.Contains(got, check) { diff --git a/cmd/calcmark/cmd/eval.go b/cmd/calcmark/cmd/eval.go index f5399ff0..1c51ada2 100644 --- a/cmd/calcmark/cmd/eval.go +++ b/cmd/calcmark/cmd/eval.go @@ -2,6 +2,7 @@ package cmd import ( "encoding/json" + "errors" "fmt" "io" "os" @@ -95,8 +96,10 @@ func runEval(args []string) error { eval := implDoc.NewEvaluator() eval.SetDisplayFormatter(localeFormatter()) - if err := eval.Evaluate(doc); err != nil { - return returnError("evaluation error: %w", err) + evalErr := eval.Evaluate(doc) + if evalErr != nil && !errors.Is(evalErr, implDoc.ErrPartialEvaluation) { + // Fatal evaluation error (e.g., frontmatter failure) — no output + return returnError("evaluation error: %w", evalErr) } // Use the selected formatter for eval output (defaults to "text") @@ -111,6 +114,13 @@ func runEval(args []string) error { return returnError("format error: %w", err) } + // If partial evaluation, output was formatted with partial results + diagnostics. + // Return error for non-zero exit code, but don't write JSON error envelope + // (the formatted output already contains diagnostic information). + if errors.Is(evalErr, implDoc.ErrPartialEvaluation) { + return fmt.Errorf("evaluation error: %w", evalErr) + } + return nil } diff --git a/cmd/calcmark/cmd/json_error_test.go b/cmd/calcmark/cmd/json_error_test.go index c6185b8e..b9a3000e 100644 --- a/cmd/calcmark/cmd/json_error_test.go +++ b/cmd/calcmark/cmd/json_error_test.go @@ -10,16 +10,6 @@ import ( "time" ) -// jsonErrorEnvelopeTest mirrors the JSON error structure we expect on stdout. -type jsonErrorEnvelopeTest struct { - Error struct { - Type string `json:"type"` - Message string `json:"message"` - Line int `json:"line,omitempty"` - Code string `json:"code,omitempty"` - } `json:"error"` -} - // TestJSONError_EvalErrorProducesJSONOnStdout verifies that when --format json // is active and an evaluation error occurs (e.g., variable redefinition), // stdout contains a valid JSON error envelope instead of being empty. @@ -44,28 +34,23 @@ func TestJSONError_EvalErrorProducesJSONOnStdout(t *testing.T) { t.Fatal("expected non-zero exit code for variable redefinition") } - // stdout must contain valid JSON + // With error recovery, stdout contains formatted JSON output (partial results + // with diagnostics), not a JSON error envelope. if !json.Valid(stdout.Bytes()) { t.Fatalf("stdout should be valid JSON when --format json is used, got:\nstdout: %q\nstderr: %q", stdout.String(), stderr.String()) } - // Parse and verify the error envelope - var envelope jsonErrorEnvelopeTest - if err := json.Unmarshal(stdout.Bytes(), &envelope); err != nil { - t.Fatalf("failed to unmarshal JSON error: %v\nstdout: %s", err, stdout.String()) - } - - if envelope.Error.Type == "" { - t.Error("error.type should not be empty") - } - if envelope.Error.Message == "" { - t.Error("error.message should not be empty") + // The output should contain the block with redefinition info + out := stdout.String() + if !strings.Contains(out, "reassign") && !strings.Contains(out, "immutable") && !strings.Contains(out, "redefinition") { + t.Errorf("JSON output should contain redefinition info, got: %s", out) } } -// TestJSONError_EvalErrorHasStructuredFields verifies that the JSON error -// envelope includes type, line, code, and message fields extracted from -// structured error messages. +// TestJSONError_EvalErrorHasStructuredFields verifies that with error recovery, +// redefinition errors produce formatted JSON output (with block diagnostics) +// and a non-zero exit code. The error details are in the block's diagnostics, +// not in a separate error envelope. func TestJSONError_EvalErrorHasStructuredFields(t *testing.T) { binary := buildCM(t) @@ -80,29 +65,33 @@ func TestJSONError_EvalErrorHasStructuredFields(t *testing.T) { cmd.Stdout = &stdout cmd.Stderr = &stderr - cmd.Run() //nolint:errcheck // we expect failure + err := cmd.Run() - var envelope jsonErrorEnvelopeTest - if err := json.Unmarshal(stdout.Bytes(), &envelope); err != nil { - t.Fatalf("failed to unmarshal JSON error: %v\nstdout: %s", err, stdout.String()) + // Should fail (non-zero exit) due to partial evaluation + if err == nil { + t.Error("Expected non-zero exit code for redefinition error") } - if envelope.Error.Type != "evaluation_error" { - t.Errorf("error.type = %q, want %q", envelope.Error.Type, "evaluation_error") - } - if envelope.Error.Code != "variable_redefinition" { - t.Errorf("error.code = %q, want %q", envelope.Error.Code, "variable_redefinition") + // stderr should mention the error + if !strings.Contains(stderr.String(), "evaluation error") { + t.Errorf("stderr should mention evaluation error, got: %q", stderr.String()) } - if envelope.Error.Line == 0 { - t.Error("error.line should be non-zero for variable_redefinition") + + // stdout should contain valid JSON output with block diagnostics + out := stdout.String() + if out == "" { + t.Fatal("stdout should not be empty — partial evaluation should still format output") } - if !strings.Contains(envelope.Error.Message, "reassign") { - t.Errorf("error.message should mention reassignment, got %q", envelope.Error.Message) + + // The output should contain information about the redefinition + if !strings.Contains(out, "reassign") && !strings.Contains(out, "redefinition") && !strings.Contains(out, "immutable") { + t.Errorf("JSON output should contain redefinition diagnostic info, got: %s", out) } } // TestJSONError_UndefinedVariableProducesJSON verifies that undefined variable -// errors also produce JSON on stdout when --format json is active. +// errors produce formatted JSON output on stdout when --format json is active, +// and a non-zero exit code. func TestJSONError_UndefinedVariableProducesJSON(t *testing.T) { binary := buildCM(t) @@ -130,19 +119,11 @@ func TestJSONError_UndefinedVariableProducesJSON(t *testing.T) { if !json.Valid(stdout.Bytes()) { t.Fatalf("stdout should be valid JSON, got:\nstdout: %q\nstderr: %q", stdout.String(), stderr.String()) } - - var envelope jsonErrorEnvelopeTest - if err := json.Unmarshal(stdout.Bytes(), &envelope); err != nil { - t.Fatalf("failed to unmarshal: %v", err) - } - - if envelope.Error.Type == "" { - t.Error("error.type should not be empty") - } } // TestJSONError_PipedStdinErrorProducesJSON verifies that the piped-stdin -// path (cm --format json, without eval subcommand) also produces JSON errors. +// path (cm --format json, without eval subcommand) produces formatted JSON +// output with diagnostics for errors, and a non-zero exit code. func TestJSONError_PipedStdinErrorProducesJSON(t *testing.T) { binary := buildCM(t) @@ -166,15 +147,6 @@ func TestJSONError_PipedStdinErrorProducesJSON(t *testing.T) { if !json.Valid(stdout.Bytes()) { t.Fatalf("stdout should be valid JSON via piped path, got:\nstdout: %q\nstderr: %q", stdout.String(), stderr.String()) } - - var envelope jsonErrorEnvelopeTest - if err := json.Unmarshal(stdout.Bytes(), &envelope); err != nil { - t.Fatalf("failed to unmarshal: %v", err) - } - - if envelope.Error.Type == "" { - t.Error("error.type should not be empty") - } } // TestJSONError_NonJSONFormatDoesNotWriteJSON verifies that errors without @@ -195,13 +167,11 @@ func TestJSONError_NonJSONFormatDoesNotWriteJSON(t *testing.T) { cmd.Run() //nolint:errcheck // we expect failure - // stdout should be empty (default text format, error only on stderr) - if stdout.Len() > 0 { - t.Errorf("stdout should be empty for text format errors, got: %q", stdout.String()) - } + // With error recovery, text output is still produced (partial results + diagnostics). + // stdout may contain formatted output. // stderr should have the error - if !strings.Contains(stderr.String(), "Error") { + if !strings.Contains(stderr.String(), "Error") && !strings.Contains(stderr.String(), "error") { t.Errorf("stderr should contain error message, got: %q", stderr.String()) } } diff --git a/cmd/calcmark/tui/editor/catwalk_error_recovery_test.go b/cmd/calcmark/tui/editor/catwalk_error_recovery_test.go new file mode 100644 index 00000000..9be6db35 --- /dev/null +++ b/cmd/calcmark/tui/editor/catwalk_error_recovery_test.go @@ -0,0 +1,63 @@ +package editor + +import ( + "fmt" + "io" + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + impldoc "github.com/CalcMark/go-calcmark/impl/document" + "github.com/CalcMark/go-calcmark/spec/document" + "github.com/cockroachdb/datadriven" +) + +// TestEditorCatwalkErrorRecoveryCascading verifies TUI display of error recovery: +// root-cause errors show as non-blocked, cascading errors show as blocked (IsBlocked=true). +func TestEditorCatwalkErrorRecoveryCascading(t *testing.T) { + content := `a = 1 / 0 +b = 10 +c = a * 2 +` + + doc, err := document.NewDocument(content) + if err != nil { + t.Fatalf("Failed to create document: %v", err) + } + + eval := impldoc.NewEvaluator() + _ = eval.Evaluate(doc) // ErrPartialEvaluation expected + + datadriven.Walk(t, "testdata", func(t *testing.T, path string) { + if !strings.HasSuffix(path, "error_recovery_cascading") { + return + } + + m := New(doc) + m.width = 80 + m.height = 24 + m.previewMode = PreviewFull + + RunModelV2(t, path, m, + WithObserverV2("results", func(out io.Writer, m tea.Model) error { + model := m.(Model) + results := model.GetLineResults() + var buf strings.Builder + for _, r := range results { + if r.IsCalc || r.Error != "" { + errorMsg := r.Error + if errorMsg == "" { + errorMsg = `""` + } else { + errorMsg = fmt.Sprintf("%q", errorMsg) + } + buf.WriteString(fmt.Sprintf("Line %d (%s): value=%s, error=%s, blocked=%v\n", + r.LineNum, r.Source, r.Value, errorMsg, r.IsBlocked)) + } + } + _, err := out.Write([]byte(buf.String())) + return err + }), + ) + }) +} diff --git a/cmd/calcmark/tui/editor/catwalk_test.go b/cmd/calcmark/tui/editor/catwalk_test.go index fad45f82..278a8404 100644 --- a/cmd/calcmark/tui/editor/catwalk_test.go +++ b/cmd/calcmark/tui/editor/catwalk_test.go @@ -100,6 +100,7 @@ z = 30` "scale_invalid_category", // TestEditorCatwalkScaleInvalidCategory "scale_invalid_view", // TestEditorCatwalkScaleInvalidView "fraction_results", // TestEditorCatwalkFractionResults + "error_recovery_cascading", // TestEditorCatwalkErrorRecoveryCascading } for _, skip := range skipTests { if strings.HasSuffix(path, skip) { diff --git a/cmd/calcmark/tui/editor/redefinition_display_test.go b/cmd/calcmark/tui/editor/redefinition_display_test.go index 1c2bf965..534f7241 100644 --- a/cmd/calcmark/tui/editor/redefinition_display_test.go +++ b/cmd/calcmark/tui/editor/redefinition_display_test.go @@ -1,6 +1,7 @@ package editor import ( + "errors" "strings" "testing" @@ -120,9 +121,25 @@ func TestRedefinitionInSingleBlock(t *testing.T) { t.Logf("Got expected error: %v", evalErr) - // The error message should mention redefinition - if !strings.Contains(evalErr.Error(), "redefinition") && - !strings.Contains(evalErr.Error(), "already defined") { - t.Errorf("Error should mention redefinition: %v", evalErr) + // With error recovery, Evaluate returns ErrPartialEvaluation. + // The specific redefinition error is on the block's diagnostics. + if !errors.Is(evalErr, impldoc.ErrPartialEvaluation) { + t.Errorf("Expected ErrPartialEvaluation, got: %v", evalErr) + } + + // Check that the block has a redefinition error + docBlocks := doc.GetBlocks() + foundRedef := false + for _, node := range docBlocks { + if cb, ok := node.Block.(*document.CalcBlock); ok { + if cb.Error() != nil && (strings.Contains(cb.Error().Error(), "redefinition") || + strings.Contains(cb.Error().Error(), "already defined") || + strings.Contains(cb.Error().Error(), "reassign")) { + foundRedef = true + } + } + } + if !foundRedef { + t.Error("Expected a block to have redefinition error in diagnostics") } } diff --git a/cmd/calcmark/tui/editor/results.go b/cmd/calcmark/tui/editor/results.go index 772a92c8..b3b3cf3e 100644 --- a/cmd/calcmark/tui/editor/results.go +++ b/cmd/calcmark/tui/editor/results.go @@ -123,8 +123,13 @@ func (m *Model) GetLineResults() []LineResult { lr.Diagnostic = diag lr.Error = diag.Code + ": " + diag.Message // Legacy string for backwards compat - // Check if this is a cascading (blocked) error - if isUndefinedVarError(lr.Error) { + // Check if this is a cascading (blocked) error. + // The evaluator marks cascading errors with diagnostic code + // "cascading_error" and hint severity. Also check the legacy + // string-match path for undefined variable errors. + if diag.Code == "cascading_error" { + lr.IsBlocked = true + } else if isUndefinedVarError(lr.Error) { varName := extractVarFromUndefinedError(lr.Error) if varName != "" && blockedVars[varName] { lr.IsBlocked = true @@ -158,8 +163,12 @@ func (m *Model) GetLineResults() []LineResult { if showErrorHere { lr.Error = blockError.Error() - // Check if this is a cascading (blocked) error - if isUndefinedVarError(lr.Error) { + // Check if this is a cascading (blocked) error. + // In the fallback path we don't have a diagnostic code, + // so check the error message for the cascading pattern. + if isCascadingErrorMessage(lr.Error) { + lr.IsBlocked = true + } else if isUndefinedVarError(lr.Error) { varName := extractVarFromUndefinedError(lr.Error) if varName != "" && blockedVars[varName] { lr.IsBlocked = true @@ -309,6 +318,13 @@ func isUndefinedVarError(errMsg string) bool { strings.Contains(lowerErr, "undefined_variable") } +// isCascadingErrorMessage checks if an error message matches the cascading error +// pattern produced by CascadingError ("depends on errored variable ..."). +// Used in the blockError fallback path where no structured diagnostic code is available. +func isCascadingErrorMessage(errMsg string) bool { + return strings.Contains(errMsg, "depends on errored variable") +} + // extractVarFromUndefinedError extracts the variable name from an undefined variable error. // Returns empty string if no variable name found. func extractVarFromUndefinedError(errMsg string) string { diff --git a/cmd/calcmark/tui/editor/results_test.go b/cmd/calcmark/tui/editor/results_test.go index 60d8c0a5..944f9d72 100644 --- a/cmd/calcmark/tui/editor/results_test.go +++ b/cmd/calcmark/tui/editor/results_test.go @@ -2,6 +2,7 @@ package editor import ( "slices" + "strings" "testing" "github.com/CalcMark/go-calcmark/spec/document" @@ -132,3 +133,135 @@ func TestIssue10_EURNotMatchedAsE(t *testing.T) { t.Errorf("expected no variable references for currency arithmetic, got %v", refs) } } + +// TestCascadingErrorDiagnosticSetsIsBlocked verifies that a cascading_error +// diagnostic code from the evaluator causes IsBlocked=true on the affected line. +func TestCascadingErrorDiagnosticSetsIsBlocked(t *testing.T) { + // Block 1: a = 1/0 (root cause error) + // Block 2: b = a * 2 (cascading error - depends on errored "a") + source := "a = 1 / 0\n\nSome text\n\nb = a * 2\n" + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + m := New(doc) + results := m.GetLineResults() + + // Find the line for "b = a * 2" + var bResult *LineResult + for i := range results { + if strings.Contains(results[i].Source, "b = a * 2") { + bResult = &results[i] + break + } + } + if bResult == nil { + t.Fatal("could not find result for 'b = a * 2'") + } + + if !bResult.IsBlocked { + t.Errorf("expected IsBlocked=true for cascading error on 'b = a * 2', got false; error=%q", bResult.Error) + } + + // Find the line for "a = 1 / 0" - root cause should NOT be blocked + var aResult *LineResult + for i := range results { + if strings.Contains(results[i].Source, "a = 1 / 0") { + aResult = &results[i] + break + } + } + if aResult == nil { + t.Fatal("could not find result for 'a = 1 / 0'") + } + if aResult.IsBlocked { + t.Error("expected IsBlocked=false for root-cause error on 'a = 1 / 0'") + } +} + +// TestCascadingErrorWithinSameBlock verifies cascading detection within a single block. +func TestCascadingErrorWithinSameBlock(t *testing.T) { + // Both statements in one block: a = 1/0 fails, b = a*2 cascading + source := "a = 1 / 0\nb = a * 2\n" + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + m := New(doc) + results := m.GetLineResults() + + var aResult, bResult *LineResult + for i := range results { + switch { + case strings.Contains(results[i].Source, "a = 1 / 0"): + aResult = &results[i] + case strings.Contains(results[i].Source, "b = a * 2"): + bResult = &results[i] + } + } + + if aResult == nil || bResult == nil { + t.Fatalf("could not find both results; a=%v b=%v", aResult, bResult) + } + + if aResult.IsBlocked { + t.Error("root-cause 'a = 1 / 0' should NOT be blocked") + } + if !bResult.IsBlocked { + t.Errorf("cascading 'b = a * 2' should be blocked; error=%q", bResult.Error) + } +} + +// TestCascadingErrorNilResultPlaceholder verifies that nil result placeholders +// (for failed statements) are handled gracefully without panics. +func TestCascadingErrorNilResultPlaceholder(t *testing.T) { + // a = 1/0 produces a nil result placeholder; b = a * 2 also nil + source := "a = 1 / 0\nb = a * 2\n" + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + m := New(doc) + // Should not panic + results := m.GetLineResults() + + // Both lines should have errors (not blank values) + for _, r := range results { + if r.IsCalc && strings.TrimSpace(r.Source) != "" { + if r.Error == "" && r.Value == "" { + t.Errorf("line %q has neither error nor value", r.Source) + } + } + } +} + +// TestMultipleCascadingErrorsAcrossBlocks verifies that cascading errors +// from multiple blocks all show as blocked. +func TestMultipleCascadingErrorsAcrossBlocks(t *testing.T) { + // Block 1: a = 1/0 + // Block 2: b = a * 2 + // Block 3: c = a + 1 + source := "a = 1 / 0\n\ntext\n\nb = a * 2\n\ntext\n\nc = a + 1\n" + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + m := New(doc) + results := m.GetLineResults() + + blocked := 0 + for _, r := range results { + if r.IsBlocked { + blocked++ + } + } + + if blocked < 2 { + t.Errorf("expected at least 2 blocked lines (b and c), got %d", blocked) + for _, r := range results { + if r.IsCalc && r.Error != "" { + t.Logf(" line=%q error=%q blocked=%v", r.Source, r.Error, r.IsBlocked) + } + } + } +} diff --git a/cmd/calcmark/tui/editor/testdata/error_recovery_cascading b/cmd/calcmark/tui/editor/testdata/error_recovery_cascading new file mode 100644 index 00000000..03f23823 --- /dev/null +++ b/cmd/calcmark/tui/editor/testdata/error_recovery_cascading @@ -0,0 +1,10 @@ +# Error recovery: root cause vs cascading errors. +# Given: a = 1 / 0 (root), b = 10 (ok), c = a * 2 (cascading, blocked) +# Expected: a shows error, b shows value, c shows blocked error. + +run observe=results +---- +-- results: +Line 0 (a = 1 / 0): value=, error="eval_error: division by zero", blocked=false +Line 1 (b = 10): value=10, error="", blocked=false +Line 2 (c = a * 2): value=, error="cascading_error: depends on errored variable \"a\": division by zero", blocked=true diff --git a/cmd/doceval/main.go b/cmd/doceval/main.go index 7561ef71..39c49803 100644 --- a/cmd/doceval/main.go +++ b/cmd/doceval/main.go @@ -19,6 +19,7 @@ import ( "bytes" "crypto/sha256" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -40,6 +41,7 @@ type BlockResult struct { type LineResult struct { Source string `json:"source"` Result string `json:"result,omitempty"` + Error string `json:"error,omitempty"` IsBlank bool `json:"is_blank,omitempty"` Variable string `json:"variable,omitempty"` } @@ -180,9 +182,13 @@ func generateFullDoc(parentMD, cmSource, parentTitle string) error { return fmt.Errorf("parse %s: %w", cmSource, err) } - eval := implDoc.NewEvaluator() - if err := eval.Evaluate(doc); err != nil { - return fmt.Errorf("eval %s: %w", cmSource, err) + evaluator := implDoc.NewEvaluator() + evalErr := evaluator.Evaluate(doc) + if evalErr != nil && !errors.Is(evalErr, implDoc.ErrPartialEvaluation) { + return fmt.Errorf("eval %s: %w", cmSource, evalErr) + } + if errors.Is(evalErr, implDoc.ErrPartialEvaluation) { + fmt.Fprintf(os.Stderr, "warning: %s: partial evaluation (%s)\n", cmSource, evalErr) } var body bytes.Buffer @@ -297,10 +303,13 @@ func evalProgressive(content string, blocks []string, results map[string]BlockRe return []string{fmt.Sprintf("parse error: %s", err)} } - eval := implDoc.NewEvaluator() - if err := eval.Evaluate(doc); err != nil { - return []string{fmt.Sprintf("eval error: %s", err)} + evaluator := implDoc.NewEvaluator() + evalErr := evaluator.Evaluate(doc) + if evalErr != nil && !errors.Is(evalErr, implDoc.ErrPartialEvaluation) { + return []string{fmt.Sprintf("eval error: %s", evalErr)} } + // On ErrPartialEvaluation, continue with partial results — + // successful lines get values, errored lines get error text via diagnostics. // Build a source-line → result map from the evaluated document. // The interpreter classifies lines into CalcBlock/TextBlock, which may @@ -356,20 +365,38 @@ func evalBlockIndependent(source string, df display.Formatter) BlockResult { return BlockResult{Error: err.Error()} } - eval := implDoc.NewEvaluator() - if err := eval.Evaluate(doc); err != nil { - return BlockResult{Error: err.Error()} + evaluator := implDoc.NewEvaluator() + evalErr := evaluator.Evaluate(doc) + if evalErr != nil && !errors.Is(evalErr, implDoc.ErrPartialEvaluation) { + return BlockResult{Error: evalErr.Error()} + } + + var partialError string + if errors.Is(evalErr, implDoc.ErrPartialEvaluation) { + partialError = evalErr.Error() } var lineResults []LineResult for _, node := range doc.GetBlocks() { switch block := node.Block.(type) { case *document.CalcBlock: + // Build diagnostic-by-line map for error display + diagByLine := make(map[int]document.Diagnostic) + for _, d := range block.Diagnostics() { + if d.Line > 0 { + diagByLine[d.Line] = d + } + } + stmts := format.AlignResults(block) + lineNum := 0 for _, stmt := range stmts { + lineNum++ // count every source line (including blanks) to match diagnostic Line numbers lr := LineResult{Source: stmt.Source, IsBlank: stmt.IsBlank, Variable: stmt.Variable} if stmt.Result != nil { lr.Result = df.Format(stmt.Result) + } else if d, ok := diagByLine[lineNum]; ok { + lr.Error = d.Message } lineResults = append(lineResults, lr) } @@ -383,7 +410,7 @@ func evalBlockIndependent(source string, df display.Formatter) BlockResult { } } - return BlockResult{Lines: lineResults} + return BlockResult{Lines: lineResults, Error: partialError} } // extractCMFrontmatter scans for ```yaml blocks containing CalcMark diff --git a/convert.go b/convert.go index bb629133..f779f9f8 100644 --- a/convert.go +++ b/convert.go @@ -112,8 +112,9 @@ func convertCM(input string, opts Options) (string, error) { eval := impldoc.NewEvaluator() eval.SetDisplayFormatter(localeFormatter(opts.Locale)) - if err := eval.Evaluate(doc); err != nil { - return "", fmt.Errorf("evaluation error: %w", err) + evalErr := eval.Evaluate(doc) + if evalErr != nil && !errors.Is(evalErr, impldoc.ErrPartialEvaluation) { + return "", fmt.Errorf("evaluation error: %w", evalErr) } formatter := format.GetFormatter(opts.Format, "") @@ -135,6 +136,10 @@ func convertCM(input string, opts Options) (string, error) { return "", fmt.Errorf("format error: %w", err) } + // Return formatted output; if partial evaluation, also return error + if errors.Is(evalErr, impldoc.ErrPartialEvaluation) { + return buf.String(), fmt.Errorf("evaluation error: %w", evalErr) + } return buf.String(), nil } @@ -216,8 +221,9 @@ func evalEmbeddedBlock(source string, openLine int, df display.Formatter) embedd eval := impldoc.NewEvaluator() eval.SetDisplayFormatter(df) - if err := eval.Evaluate(doc); err != nil { - return embeddedBlockResult{text: formatEmbeddedBlockError(err.Error(), contentLine), isErr: true} + evalErr := eval.Evaluate(doc) + if evalErr != nil && !errors.Is(evalErr, impldoc.ErrPartialEvaluation) { + return embeddedBlockResult{text: formatEmbeddedBlockError(evalErr.Error(), contentLine), isErr: true} } var buf bytes.Buffer @@ -231,7 +237,7 @@ func evalEmbeddedBlock(source string, openLine int, df display.Formatter) embedd return embeddedBlockResult{text: formatEmbeddedBlockError(err.Error(), contentLine), isErr: true} } - return embeddedBlockResult{text: buf.String()} + return embeddedBlockResult{text: buf.String(), isErr: errors.Is(evalErr, impldoc.ErrPartialEvaluation)} } // formatEmbeddedBlockError returns an inline error blockquote for a failed CalcMark block. diff --git a/docs/plans/2026-04-06-002-feat-evaluator-error-recovery-plan.md b/docs/plans/2026-04-06-002-feat-evaluator-error-recovery-plan.md new file mode 100644 index 00000000..a2a0272e --- /dev/null +++ b/docs/plans/2026-04-06-002-feat-evaluator-error-recovery-plan.md @@ -0,0 +1,521 @@ +--- +title: "feat: error recovery in evaluator for multi-diagnostic support" +type: feat +status: completed +pr: https://github.com/CalcMark/go-calcmark/pull/112 +date: 2026-04-06 +origin: https://github.com/CalcMark/go-calcmark/issues/73 +--- + +# feat: error recovery in evaluator for multi-diagnostic support + +## Overview + +The CalcMark evaluator currently stops at the first error, meaning the LSP, TUI, and CLI can only surface one diagnostic at a time. This plan adds error recovery so the evaluator continues past failed statements and blocks, tracking errored variables out-of-band and distinguishing cascading errors (hint severity) from root-cause errors. The result: users see all diagnostics simultaneously, with successful computations still showing values. + +## Problem Frame + +A 50-line CalcMark document with an error on line 3 hides all feedback for the remaining 47 lines. Users must fix errors sequentially instead of seeing the full picture. This was discovered during LSP development (#72) and affects the TUI editor equally. The limitation is in the evaluator, not the downstream consumers. + +## Requirements Trace + +- R1. Multiple errors in a document are all visible simultaneously +- R2. Variables with successful evaluation show their values even when other lines have errors +- R3. Cascading errors (from errored dependencies) are visually distinct from root-cause errors +- R4. Works in LSP (`cm lsp`), TUI editor, and CLI +- R5. No performance regression for valid documents +- R6. Existing tests continue to pass + +## Scope Boundaries + +- **In scope:** `Evaluator.Evaluate()`, `EvaluateBlock()`, `EvaluateAffectedBlocks()`, and their consumers (TUI results, CLI eval, formatters) +- **In scope:** `cmd/doceval` (Lark website doc evaluator) and `convert.go` (Go library API) -- both call `Evaluate()` and must handle `ErrPartialEvaluation` +- **Out of scope:** `calcmark.Eval()` public Go API (separate evaluation pipeline that bypasses `Evaluator`, file follow-up issue) +- **Out of scope:** Parse error recovery within the parser itself (if parsing fails, the whole block is errored) +- **Out of scope:** Semantic error recovery within a block (semantic errors still mark the whole block) + +## Context & Research + +### Relevant Code and Patterns + +- `impl/document/evaluator.go` -- Three entry points (`Evaluate`, `EvaluateBlock`, `EvaluateAffectedBlocks`) all share the same stop-on-first-error pattern via `evaluateCalcBlockWithDoc` +- `impl/interpreter/environment.go` -- Simple `map[string]types.Type` for variable bindings. No sentinel/error concept yet +- `spec/document/document.go` -- `Diagnostic` struct already supports `"hint"` severity +- `cmd/calcmark/tui/editor/results.go` -- Already tracks `blockedVars` and `IsBlocked` for cascading errors within a block via string-matching on error messages +- `lsp/server.go:251` -- Already ignores `Evaluate()` error return and collects all block diagnostics +- `lsp/diagnostics.go` -- Already maps `"hint"` severity to `DiagnosticSeverityHint` + +### Institutional Learnings + +- **Statement index drift** (`docs/solutions/ui-bugs/context-footer-statement-index-drift.md`): 4 prior incidents where source lines and result indices get misaligned. Prevention: use nil placeholders in results slice so `len(results) == len(statements)`. Use separate `resultIdx` counters when iterating source lines vs results +- **Diagnostic pipeline** (`docs/solutions/code-organization/diagnostic-detailed-field-pipeline.md`): Every diagnostic must carry `Detailed` field through the full pipeline (semantic -> document -> calcmark). Three carry sites in the evaluator +- **DocLine line numbers** (`docs/solutions/code-organization/docline-diagnostic-line-numbers.md`): Use `blockLineOffset()` for document-absolute line numbers, `node.GetRange()` for block-relative. Never type-assert to specific AST node types +- **Defense-in-depth** (`docs/solutions/security-issues/nan-inf-panic-yaml-frontmatter-scale.md`): Error recovery means more code paths reach the interpreter. Sentinel detection must happen before crash-prone operations. Fuzz test invariant ("never panic on any input") must hold + +## Key Technical Decisions + +- **Out-of-band error tracking:** Add `erroredVars map[string]error` to `Environment` rather than a sentinel `types.Type`. This minimizes blast radius -- no changes to the type system, formatters, or transforms. The interpreter checks `env.GetError(name)` in `evalIdentifier` before the main variable lookup +- **Statement-level + block-level recovery:** Continue past failed statements within a block AND continue to next blocks. Use nil placeholders in the results slice to maintain 1:1 alignment with statements (prevents statement index drift) +- **Sentinel return value:** `Evaluate()` returns a new `ErrPartialEvaluation` when some blocks had errors but evaluation continued. Callers use `errors.Is()` to detect this. CLI treats it as exit 1 but still formats output. LSP already ignores the error +- **Cascading error detection:** Use a structured `cascading_error` diagnostic code with `Hint` severity. The `Detailed` field names the root-cause variable. The TUI's `results.go` checks for this code alongside the existing `undefined_variable` string matching +- **Block dirty state:** A block with any errors is NOT marked clean (`SetDirty(false)` only on full success). Partial results are stored but the block remains dirty for re-evaluation +- **`@global`/`@exchange` directives:** If a block has directives that succeed but later statements fail, the directive mutations persist. This is correct -- the directive executed successfully + +## Open Questions + +### Resolved During Planning + +- **Should parse errors allow partial block evaluation?** No. Parse errors mean no valid AST exists. The block gets a diagnostic but no statements can run. Other blocks continue +- **Should semantic errors allow partial interpretation?** No. Semantic errors (redefined variable, type mismatch) currently return before interpretation. Keeping this behavior avoids complexity. Other blocks continue +- **What should `{{var}}` render for errored variables?** Leave as `{{var}}` (unresolved). The variable is not in the environment's `vars` map, so the existing unresolved-variable handling applies naturally +- **Should errored variables count as "defined" for redefinition checks?** Yes. An errored variable was assigned; a later block redefining it is still a redefinition error + +### Deferred to Implementation + +- Exact error message format for cascading errors (must work with existing `isUndefinedVarError` string matching in TUI, or TUI detection updated simultaneously) +- Performance impact of nil-placeholder results on formatter alignment code (note: `format/align.go:67` already nil-guards results) + +## High-Level Technical Design + +> *This illustrates the intended approach and is directional guidance for review, not implementation specification. The implementing agent should treat it as context, not code to reproduce.* + +``` +Evaluate(doc) flow with error recovery: + + ApplyFrontmatter ──error?──> return err (fatal, no recovery) + │ + ▼ + for each block: + ├── TextBlock ──> checkTextBlockForLikelyCalculations (unchanged) + │ + └── CalcBlock ──> evaluateCalcBlockWithDoc + │ + ├── Parse ──error?──> add diagnostic, block has no AST + │ (can't identify vars to mark) + │ continue to next block + │ + ├── Semantic check ──error?──> add diagnostic, + │ mark defined vars errored + │ (extracted from AST) + │ continue to next block + │ + └── Interpret stmt-by-stmt: + │ + ├── interp.Eval returns CascadingError? + │ (detected in evalIdentifier via env.GetError) + │ ──> cascading_error diag (hint severity) + │ nil placeholder in results + │ mark assigned var errored + │ continue to next stmt + │ + ├── eval error (div/0, type mismatch)? + │ ──> eval_error diag + │ nil placeholder in results + │ mark assigned var errored + │ continue to next stmt + │ + └── success ──> append result + set var in env + + applyTransforms (skip blocks with errors) + interpolateTextBlocks (errored vars stay as {{var}}) + + return ErrPartialEvaluation if any block had errors, else nil +``` + +**Environment error tracking:** +``` +Environment { + vars: map[string]types.Type // successful values + erroredVars: map[string]error // errored variable tracking + ... +} + +SetError(name, err) -- marks variable as errored +GetError(name) -> (error, bool) -- checks if variable is errored +HasError(name) -> bool -- quick check +ClearErrors() -- reset for fresh evaluation +``` + +## Implementation Units + +- [ ] **Unit 1: Add error tracking to Environment** + +**Goal:** Extend `Environment` with out-of-band error tracking for variables that failed evaluation. + +**Requirements:** R1, R2, R3 + +**Dependencies:** None + +**Files:** +- Modify: `impl/interpreter/environment.go` +- Test: `impl/interpreter/environment_test.go` + +**Approach:** +- Add `erroredVars map[string]error` field to `Environment` +- Add methods: `SetError(name string, err error)`, `GetError(name string) (error, bool)`, `ClearError(name string)`, `ClearErrors()` +- `ClearError(name)` removes a single variable's error state (needed for incremental re-evaluation via `EvaluateAffectedBlocks` when a previously errored variable is fixed) +- Initialize map in `NewEnvironment()` +- `Clone()` must copy the `erroredVars` map +- `GetAllVariables()` is unchanged -- it returns only successful vars + +**Patterns to follow:** +- Existing `vars`/`exchangeRates` map patterns in `environment.go` +- `Clone()` uses `maps.Copy` for both existing maps + +**Test scenarios:** +- Happy path: `SetError("x", err)` then `GetError("x")` returns the error and true +- Happy path: `GetError("x")` returns nil, false for non-errored variables +- Happy path: `ClearError("x")` removes a single variable's error; `GetError("x")` returns nil, false after +- Happy path: `ClearErrors()` removes all tracked errors +- Edge case: `SetError` then `Set` (successful value) -- both coexist, `GetError` still returns true (variable is both errored and has a prior value from a different context) +- Edge case: `Clone()` includes errored vars -- mutating clone does not affect original +- Edge case: `GetError` on unset variable returns nil, false + +**Verification:** +- All existing environment tests pass unchanged +- New error tracking methods work independently from the vars map + +--- + +- [ ] **Unit 2: Add ErrPartialEvaluation sentinel and cascading_error diagnostic code** + +**Goal:** Define the sentinel error type returned by `Evaluate()` and the diagnostic code used for cascading errors. + +**Requirements:** R1, R3, R4 + +**Dependencies:** None (can be done in parallel with Unit 1) + +**Files:** +- Modify: `impl/document/evaluator.go` (add sentinel error and diagnostic code constants near top of file) + +**Approach:** +- Define `var ErrPartialEvaluation = errors.New("partial evaluation: one or more blocks had errors")` in evaluator.go +- Document `cascading_error` as the diagnostic code for errors caused by referencing an errored variable +- The `Detailed` field on cascading error diagnostics should name the root-cause variable for user-facing display + +**Patterns to follow:** +- Existing error patterns in evaluator.go +- Existing diagnostic codes like `"parse_error"`, `"eval_error"` in `evaluateCalcBlockWithDoc` +- `Hint` severity already used for currency disambiguation in semantic checker + +**Test scenarios:** +- Happy path: `errors.Is(ErrPartialEvaluation, ErrPartialEvaluation)` returns true +- Happy path: `errors.Is(fmt.Errorf("wrapped: %w", ErrPartialEvaluation), ErrPartialEvaluation)` returns true + +**Verification:** +- Sentinel error is importable and works with `errors.Is()` + +--- + +- [ ] **Unit 3: Statement-level error recovery in evaluateCalcBlockWithDoc** + +**Goal:** Make the core per-block evaluation continue past failed statements, producing nil placeholders for failed statements and marking errored variables in the environment. + +**Requirements:** R1, R2, R3 + +**Dependencies:** Unit 1, Unit 2 + +**Files:** +- Modify: `impl/document/evaluator.go` (the `evaluateCalcBlockWithDoc` method, lines 476-637) +- Test: `impl/document/evaluator_test.go` + +**Approach:** +- In the statement evaluation loop (lines 585-595), instead of `break` on error: + - Create a diagnostic (eval_error or cascading_error with appropriate severity) + - Append nil to results slice as placeholder (maintains 1:1 alignment with statements) + - If the statement is an assignment, call `env.SetError(varName, err)` to mark the variable as errored + - Continue to next statement +- Before evaluating each statement, check if any referenced variables are errored (via `env.GetError`). If so, produce a `cascading_error` diagnostic with `Hint` severity, append nil placeholder, mark assigned var as errored, continue +- After the loop, if any statement failed, do NOT call `SetDirty(false)` -- block stays dirty +- Still set `block.SetError()` with the first error for legacy compatibility +- Still store partial results via `block.SetResults(results)` -- results slice now has nil entries for failed statements +- The method keeps its `error` return signature unchanged. It still returns an error for the first failure (used by `block.SetError()` for legacy consumers). Callers in Unit 4 check `block.Error() != nil` after the call to determine if the block had errors, track `hasErrors`, and continue to the next block. No signature change needed +- `SetLastValue` must use the last *non-nil* result, or be skipped entirely when all results are nil, to avoid downstream nil dereferences + +**Key concern -- statement index drift:** The results slice MUST have exactly `len(nodes)` entries. Each successful statement appends its result. Each failed statement appends nil. This prevents the 5th instance of statement index drift + +**Key concern -- cascading detection in interpreter:** When `interp.Eval` is called for a statement that references an errored variable, the interpreter's `evalIdentifier` needs to check `env.GetError(name)` and return a distinguishable error. This happens naturally if the variable is not in `env.vars` (undefined variable error), but we need the cascading_error distinction. Two options: (a) pre-scan the statement's AST for references to errored vars before calling interp.Eval, or (b) have the interpreter return a typed error that the evaluator can detect. Option (b) is cleaner + +**Patterns to follow:** +- Existing diagnostic creation pattern at lines 604-631 (use `node.GetRange()`, set `Line`, `DocLine` via `blockLineOffset`) +- `Detailed` field must be populated per the diagnostic pipeline learning + +**Test scenarios:** +- Happy path: Block with `a = 1 / 0; b = 10` -- both get results (nil for a, 10 for b) +- Happy path: Block with `a = 1 / 0; c = a * 2` -- a gets eval_error, c gets cascading_error with hint severity +- Happy path: Block with all successful statements -- unchanged behavior, no regressions +- Edge case: Block where every statement fails -- all nil results, all diagnostics, block stays dirty +- Edge case: Statement referencing multiple errored variables -- one cascading_error diagnostic naming one root cause +- Edge case: Whitespace/empty lines between statements -- nil placeholder not inserted for empty lines (they aren't in the nodes slice) +- Edge case: `@global x = 10` succeeds, next statement fails -- x is still set in frontmatter +- Integration: Diagnostics have correct `Line`, `DocLine`, `Column` values using `node.GetRange()` and `blockLineOffset()` +- Integration: `Detailed` field populated on cascading_error diagnostics + +**Verification:** +- `len(block.Results()) == len(block.Statements())` invariant holds for all test cases +- All existing evaluator tests pass (no behavioral change for single-error documents) +- Block with errors has `block.Error() != nil` and `block.IsDirty() == true` + +--- + +- [ ] **Unit 4: Block-level error recovery in Evaluate, EvaluateBlock, EvaluateAffectedBlocks** + +**Goal:** Make all three evaluation entry points continue to the next block when a block has errors, collecting all diagnostics across the document. + +**Requirements:** R1, R2, R4 + +**Dependencies:** Unit 3 + +**Files:** +- Modify: `impl/document/evaluator.go` (`Evaluate` lines 93-137, `EvaluateBlock` lines 193-301, `EvaluateAffectedBlocks` lines 314-337, `evaluateCalcBlockSelective` lines 342-466) +- Test: `impl/document/evaluator_test.go` + +**Approach:** +- **`Evaluate()`:** Change the CalcBlock case (lines 118-123) to NOT return on error. Instead, track `hasErrors bool`. After all blocks, return `ErrPartialEvaluation` if `hasErrors` is true, nil otherwise. `applyTransforms` and `interpolateTextBlocks` still run -- transforms already skip errored blocks (`cb.Error() != nil`), and interpolation leaves `{{var}}` unresolved for missing vars. Callers check `block.Error() != nil` after `evaluateCalcBlockWithDoc` returns to determine if the block had errors +- **`EvaluateBlock()`:** Pass 1 (lines 200-260) has two early-return paths that must become non-fatal: parse errors (lines 215-218) and variable redefinition errors (lines 243-247). Both should add diagnostics to the block and continue to the next block rather than returning. Errored variables should still be tracked in `allDefinedVars` (they count as "defined" for redefinition checks, and `cb.Variables()` from dependency analysis is already populated even if evaluation fails) +- **`EvaluateBlock()` pass 2:** `evaluateCalcBlockSelective` (lines 342-466) needs its own statement-level recovery logic parallel to Unit 3. Key difference: it uses a *cloned* environment (`evalEnv`) and selectively writes back only "authoritative" assignments. When a variable is errored, the error must propagate back to the shared `reactiveEnv` via `env.SetError(varName, err)` in the selective writeback loop (lines 454-462), so downstream blocks see the error. On successful re-evaluation of a previously errored variable, call `env.ClearError(varName)` to remove stale error state +- **`EvaluateAffectedBlocks()`:** Continue past block errors (lines 321-326). Track `hasErrors` and return `ErrPartialEvaluation` if any block failed. Since the environment is NOT reset, use `env.ClearError(varName)` when a statement succeeds that was previously errored, to support incremental fixing + +**Patterns to follow:** +- The existing block iteration loop structure +- `applyTransforms` already guards on `cb.Error() != nil` + +**Test scenarios:** +- Happy path: Document with 3 blocks, block 1 has error, blocks 2-3 are independent -- blocks 2-3 evaluate successfully, all 3 blocks have diagnostics/results +- Happy path: Document with no errors -- returns nil (unchanged behavior) +- Happy path: `Evaluate` returns `ErrPartialEvaluation` when any block has errors +- Edge case: Block 2 references errored variable from block 1 -- block 2 gets cascading_error diagnostics +- Edge case: All blocks have errors -- `ErrPartialEvaluation` returned, all blocks have diagnostics +- Edge case: `EvaluateBlock` pass 1 encounters error in block 1, pass 2 re-evaluates block 2 which depends on block 1's errored variable +- Edge case: `EvaluateAffectedBlocks` with one errored block and one clean block +- Integration: `applyTransforms` skips errored blocks but transforms successful ones +- Integration: `interpolateTextBlocks` resolves `{{var}}` for successful vars, leaves unresolved for errored vars + +**Verification:** +- `errors.Is(err, ErrPartialEvaluation)` for documents with errors +- `err == nil` for documents without errors +- All blocks have diagnostics populated regardless of which block failed first +- Existing tests continue to pass + +--- + +- [ ] **Unit 5: Interpreter cascading error detection** + +**Goal:** Make the interpreter produce a distinguishable error when a statement references an errored variable, so the evaluator can classify cascading vs root-cause diagnostics. + +**Requirements:** R3 + +**Dependencies:** Unit 1 + +**Files:** +- Modify: `impl/interpreter/variables.go` (`evalIdentifier`) +- Modify: `impl/interpreter/errors.go` (new file, ~10 lines for `CascadingError` type) +- Test: `impl/interpreter/interpreter_test.go` + +**Approach:** +- In `evalIdentifier` (variables.go:22), add a check for `env.GetError(name)` *before* `env.Get(name)`. If errored, return a `CascadingError{VarName: name, Cause: err}` instead of the generic "undefined variable" error +- `CascadingError` is a minimal struct (~5 lines): `VarName string`, `Cause error`, implements `error` interface and `Unwrap() error` +- The evaluator in Unit 3 uses `errors.As(err, &CascadingError{})` to choose between `cascading_error` (Hint) vs `eval_error` (Error) diagnostic codes +- **Alternative considered:** Pre-scanning each statement's AST against `env.erroredVars` using the existing `extractIdentifiers` pattern from `spec/document/deps.go`. Rejected because: (a) `extractIdentifiers` is unexported in the spec package, (b) duplicating AST walking logic adds more code than a 5-line typed error, (c) the interpreter is the natural place to detect "I tried to use a variable and it was errored" + +**Patterns to follow:** +- Existing error handling in `evalIdentifier` for undefined variables (variables.go:35) +- The check order matters: errored vars checked first, then `env.Get`, then boolean keywords, then "undefined variable" + +**Test scenarios:** +- Happy path: `env.SetError("x", someErr)` then evaluating expression `x + 1` returns `CascadingError` with VarName "x" +- Happy path: Normal variable lookup (not errored) works unchanged +- Edge case: Variable errored but also has a value in vars -- `GetError` takes precedence (errored check is first) +- Edge case: Expression `a + b` where `a` is errored and `b` is valid -- first `CascadingError` (for `a`) surfaces + +**Verification:** +- `errors.As(err, &CascadingError{})` correctly identifies cascading errors +- `CascadingError.Unwrap()` returns the original cause +- Non-cascading interpreter errors are unaffected + +--- + +- [ ] **Unit 6: Update TUI cascading error detection in results.go** + +**Goal:** Make the TUI's `GetLineResults` detect cascading errors from the new `cascading_error` diagnostic code, not just string-matching on "undefined variable". + +**Requirements:** R3, R4 + +**Dependencies:** Unit 3 (needs cascading_error diagnostics to exist) + +**Files:** +- Modify: `cmd/calcmark/tui/editor/results.go` +- Test: `cmd/calcmark/tui/editor/results_test.go` (or catwalk tests in `testdata/`) + +**Approach:** +- In the diagnostic-checking code (around lines 126-141, 161-176), add a check for `diag.Code == "cascading_error"` alongside the existing `isUndefinedVarError` check +- When `cascading_error` is detected, set `lr.IsBlocked = true` +- The `blockedVars` map should be seeded from all blocks' errored variables (cross-block), not just tracked incrementally within the current iteration +- Handle nil placeholders in the results slice -- when `stmtResults[stmtIdx]` is nil, treat it as an error line + +**Key concern -- statement index drift:** The results slice now has nil entries for failed statements. The existing `stmtIdx < len(stmtResults)` check handles this, but the code that reads `stmtResults[stmtIdx]` must handle nil. Verify that `results.go` checks for nil before calling `.String()` or `.TypeName()` on results + +**Patterns to follow:** +- Existing `blockedVars` tracking pattern in results.go +- Existing `diagByLine` map pattern for per-line diagnostics + +**Test scenarios:** +- Happy path: Block 1 has root-cause error, block 2 has cascading error -- block 2's error shows as `IsBlocked = true` +- Happy path: Block with `a = 1/0; b = a*2` -- line for `b` shows as blocked +- Edge case: Block with nil result placeholder -- displays error, not a blank value +- Edge case: Multiple cascading errors in different blocks all show as blocked +- Integration: TUI renders cascading errors with muted/hint styling vs prominent error styling for root causes + +**Verification:** +- `IsBlocked` is true for cascading errors, false for root-cause errors +- No statement index drift (values align with correct source lines) + +--- + +- [ ] **Unit 7: Update CLI and formatter error handling** + +**Goal:** Make `cm eval` handle `ErrPartialEvaluation` by formatting output AND exiting with code 1. + +**Requirements:** R4 + +**Dependencies:** Unit 4 + +**Files:** +- Modify: `cmd/calcmark/cmd/eval.go` +- Modify: `convert.go` (if applicable) +- Test: `cmd/calcmark/cmd/eval_test.go` (or integration test) + +**Approach:** +- In the CLI eval command, check `errors.Is(err, ErrPartialEvaluation)`. If true, format the output normally (partial results + diagnostics are on the blocks), then exit with code 1 +- For non-partial errors (frontmatter failure, etc.), keep current behavior (error message, exit 1, no output) +- **Audit all formatters for nil-safety:** `json_formatter.go`, `text_formatter.go`, `html_formatter.go`, `markdown_formatter.go` all iterate block results. With nil placeholders for failed statements, every `results[i].String()` and `results[i].TypeName()` call must guard against nil. Add nil guards where missing +- JSON formatter: blocks with errors should include `diagnostics` in the JSON output. Check existing `block.Error()` handling -- it may already work since blocks now have both results and errors +- `convert.go`: if `ErrPartialEvaluation`, still return the formatted output (with error markers), or return error. Depends on use case -- the Hugo doc evaluator should probably still fail + +**Patterns to follow:** +- Existing error handling in `cmd/calcmark/cmd/eval.go` +- JSON formatter's existing `block.Error()` checks + +**Test scenarios:** +- Happy path: `cm eval` on document with errors produces formatted output AND non-zero exit +- Happy path: `cm eval --format json` on document with errors produces JSON with both results and diagnostics +- Happy path: `cm eval` on valid document -- unchanged behavior (exit 0, full output) +- Edge case: All blocks errored -- output shows all diagnostics, exit 1 +- Error path: Frontmatter error -- still fatal, no output, exit 1 + +**Verification:** +- CLI exit code is 1 when `ErrPartialEvaluation` +- JSON output includes both results and diagnostics for mixed documents +- Valid documents are completely unaffected + +--- + +- [ ] **Unit 8: Update doceval (Lark website) and convert.go for error recovery** + +**Goal:** Make the Lark documentation site evaluator and the Go library `Convert()` API handle `ErrPartialEvaluation` correctly. The Lark site is the primary user-facing surface for CalcMark and a case study for Go API consumers. + +**Requirements:** R4 + +**Dependencies:** Unit 4 + +**Files:** +- Modify: `cmd/doceval/main.go` (3 call sites: `evaluateFullDoc` line 184, `evaluateBlockProgressive` line 301, `evaluateBlock` line 360) +- Modify: `convert.go` (2 call sites: `convertCM` line 115, `convertEmbeddedBlock` line 219) +- Test: `cmd/doceval/main_test.go` (if exists), `convert_test.go` + +**Approach:** +- **doceval:** The three `Evaluate()` call sites each handle errors differently: + - `evaluateFullDoc` (line 184): Generates full Markdown pages from .cm files. On `ErrPartialEvaluation`, should still format output with partial results -- the doc page should render with visible error markers rather than failing entirely. Log a warning with the filename and error count + - `evaluateBlockProgressive` (line 301): Progressive evaluation of ```calcmark blocks. On `ErrPartialEvaluation`, return partial results (successful lines get values, errored lines get error text). This maintains the per-line `LineResult` structure + - `evaluateBlock` (line 355): Standalone block evaluation. On `ErrPartialEvaluation`, return `BlockResult` with both `Lines` (partial results) and per-line errors +- **convert.go `convertCM`** (line 115): On `ErrPartialEvaluation`, still format the output (blocks have partial results) but also return the error to the caller. `calcmark.Convert()` callers need to know evaluation was partial. This is a behavior change: previously it returned `("", error)`, now it returns `(formatted_output, ErrPartialEvaluation)`. Callers can check `errors.Is` to decide whether to use the partial output +- **convert.go `convertEmbeddedBlock`** (line 219): On `ErrPartialEvaluation`, render the block with error markers using the existing `formatEmbeddedBlockError` pattern. The embedded block should show which lines succeeded and which failed +- Both doceval and convert.go currently iterate block results after evaluation -- nil placeholders from Unit 3 must be handled (nil results -> error display or empty) + +**Patterns to follow:** +- Existing `BlockResult.Error` field in doceval already supports per-block error reporting +- `formatEmbeddedBlockError` in convert.go already renders error markers in embedded blocks + +**Test scenarios:** +- Happy path: doceval processes a page with one errored block and one valid block -- valid block renders correctly, errored block shows diagnostics +- Happy path: convert.go produces HTML with both results and error markers for mixed documents +- Edge case: doceval progressive mode with errored variable carrying across blocks +- Edge case: convert.go embedded mode with ```calcmark block that has cascading errors +- Integration: `task generate-docs` still succeeds with the existing Lark site content (no regressions in docs) + +**Verification:** +- Lark site documentation pages render correctly with `task generate-docs` +- No panics from nil result values in doceval's `LineResult` building +- convert.go produces meaningful output (not empty) for documents with partial errors + +--- + +- [ ] **Unit 9: Golden test files and integration validation** + +**Goal:** Add comprehensive golden test files covering multi-error scenarios and validate full-stack behavior. + +**Requirements:** R1, R2, R3, R5, R6 + +**Dependencies:** Units 3-8 + +**Files:** +- Create: `testdata/eval/errors/multi_error_recovery.cm` +- Create: `testdata/eval/errors/cascading_errors.cm` +- Create: `testdata/eval/errors/mixed_success_and_errors.cm` +- Test: `impl/document/evaluator_test.go` (integration tests using golden files) + +**Approach:** +- Write golden test files exercising the key scenarios from the issue's acceptance criteria +- Multi-block document: block 1 error, block 2 references block 1, block 3 independent +- Within-block: `a = 1/0; b = 10; c = a * 2; total = a + b + c` +- All-error document: every block has errors +- Run `task test` to validate all existing tests pass +- Run `task quality` for full quality gate +- Benchmark valid document evaluation to confirm no performance regression + +**Patterns to follow:** +- Existing golden files in `testdata/eval/errors/` and `testdata/eval/success/` +- Test helper patterns in `evaluator_test.go` + +**Test scenarios:** +- Happy path: Multi-block error recovery golden file matches expected diagnostics +- Happy path: Cascading errors golden file shows hint-severity diagnostics for dependent variables +- Happy path: Mixed success/error golden file shows values for successful lines alongside errors +- Integration: All existing golden tests pass unchanged +- Integration: `task test` passes with zero failures +- Integration: `task quality` passes + +**Verification:** +- `task test` passes +- `task quality` passes +- No performance regression for valid documents (evaluator still returns nil, no extra work) + +## System-Wide Impact + +- **Interaction graph:** `evaluateCalcBlockWithDoc` -> `Environment.SetError` -> `evalIdentifier` checks error -> cascading diagnostic -> `results.go` `IsBlocked` detection -> TUI view renders hint-style. LSP path: `Evaluate` -> block diagnostics -> `publishDiagnostics` (no changes needed) +- **Error propagation:** Root-cause errors stay as `error` severity. Cascading errors propagate as `hint` severity with `cascading_error` code. `ErrPartialEvaluation` wraps the summary for callers +- **State lifecycle risks:** The `erroredVars` map on Environment persists across blocks within one `Evaluate()` call. It is cleared in `Evaluate()` when `env` is reset. For `EvaluateBlock`'s two-pass approach, errored vars from pass 1 must be available in pass 2 +- **API surface parity:** CLI, TUI, LSP, doceval (Lark site), and `convert.go` all consume the same evaluator. JSON formatter needs to handle blocks with both results and errors. doceval has 3 call sites with different error semantics. `calcmark.Eval()` public API is out of scope (follow-up issue) +- **Integration coverage:** Unit tests alone won't prove the full diagnostic pipeline. Golden tests (Unit 8) validate end-to-end +- **Unchanged invariants:** The `types.Type` interface is NOT modified. The `Diagnostic` struct is NOT modified (uses existing fields). Semantic checker behavior is unchanged. Parser behavior is unchanged + +## Risks & Dependencies + +| Risk | Mitigation | +|------|------------| +| Statement index drift (5th instance) | Nil placeholders in results slice maintain 1:1 with statements. Explicit test for `len(results) == len(statements)` invariant | +| Panic on nil result in formatters/TUI | Audit every `results[i].String()` and `results[i].TypeName()` call. Add nil guard tests | +| `EvaluateBlock` two-pass complexity | Test both passes independently. Errored vars from pass 1 carry into pass 2 via shared Environment | +| Backwards compatibility for CLI scripts | `ErrPartialEvaluation` ensures non-zero exit code. Scripts checking `$?` see failure as before | +| Lark site doc generation breaks | `task generate-docs` validation in Unit 9. doceval call sites updated in Unit 8 to handle partial results | +| Performance regression from continued evaluation | CalcMark docs are small (<1000 lines). Even evaluating all blocks after errors adds <5ms. Benchmark in Unit 8 confirms | + +## Sources & References + +- Related issue: #73 (error recovery in evaluator for multi-diagnostic support) +- Related issue: #72 (LSP development, where this limitation was discovered) +- Institutional learning: `docs/solutions/ui-bugs/context-footer-statement-index-drift.md` (4 prior incidents) +- Institutional learning: `docs/solutions/code-organization/diagnostic-detailed-field-pipeline.md` +- Institutional learning: `docs/solutions/code-organization/docline-diagnostic-line-numbers.md` +- Institutional learning: `docs/solutions/security-issues/nan-inf-panic-yaml-frontmatter-scale.md` diff --git a/format/html_formatter.go b/format/html_formatter.go index 52853a94..822237e5 100644 --- a/format/html_formatter.go +++ b/format/html_formatter.go @@ -68,19 +68,22 @@ func (f *HTMLFormatter) Extensions() []string { // TemplateBlock represents a block for template rendering type TemplateBlock struct { - Type string - SourceLines []TemplateLine // For calc blocks with per-line results - Error string - Warnings []string // Diagnostic warnings (e.g., reserved keyword as variable name) - HTML template.HTML // For text blocks - DocLine int // 1-indexed document-absolute start line (for scroll sync) + Type string + SourceLines []TemplateLine // For calc blocks with per-line results + Error string + HasLineDiagnostic bool // True if any SourceLine has a per-line Error + Warnings []string // Diagnostic warnings (e.g., reserved keyword as variable name) + HTML template.HTML // For text blocks + DocLine int // 1-indexed document-absolute start line (for scroll sync) } // TemplateLine represents a single source line with its result type TemplateLine struct { - Source string - Result string // Formatted result for this line - DocLine int // 1-indexed document-absolute line number (for scroll sync) + Source string + Result string // Formatted result for this line + Error string // Error message for this line (empty if successful) + IsCascading bool // True if this is a cascading error (hint severity) + DocLine int // 1-indexed document-absolute line number (for scroll sync) } // TemplateFrontmatter represents frontmatter for template rendering @@ -217,19 +220,31 @@ func (f *HTMLFormatter) Format(w io.Writer, doc *document.Document, opts Options tb.Type = "calculation" tb.DocLine = blockStartLine + // Build diagnostic-by-line map for per-line error display + diagByLine := make(map[int]document.Diagnostic) + for _, d := range block.Diagnostics() { + if d.Line > 0 { + diagByLine[d.Line] = d + } + } + stmts := AlignResults(block) - lineIdx := 0 + lineNum := 0 for _, stmt := range stmts { - lineIdx++ // every aligned statement is a source line + lineNum++ // count every source line (including blanks) to match diagnostic Line numbers if stmt.IsBlank || stmt.IsResultLine { continue } tl := TemplateLine{ Source: stmt.Source, - DocLine: blockStartLine + lineIdx - 1, + DocLine: blockStartLine + lineNum - 1, } if stmt.Result != nil { tl.Result = df.Format(stmt.Result) + } else if d, ok := diagByLine[lineNum]; ok { + tl.Error = d.Message + tl.IsCascading = d.Code == "cascading_error" + tb.HasLineDiagnostic = true } tb.SourceLines = append(tb.SourceLines, tl) } diff --git a/format/html_formatter_test.go b/format/html_formatter_test.go index 06ce5d8e..47d78c3c 100644 --- a/format/html_formatter_test.go +++ b/format/html_formatter_test.go @@ -747,3 +747,83 @@ func TestHTMLFormatterDataSourceLineWithFrontmatter(t *testing.T) { t.Errorf("Expected calc line with data-source-line=\"6\" after frontmatter, output:\n%s", output) } } + +// TestHTMLFormatter_ErrorAfterBlankLine verifies that an error on a line +// after a blank line within a calc block shows an inline error in the HTML. +// Diagnostic Line numbers count all source lines (including blanks), so the +// formatter's line counter must also count blanks to match diagnostics correctly. +func TestHTMLFormatter_ErrorAfterBlankLine(t *testing.T) { + // Blank line between b and a means they're in the same block but + // the diagnostic for a=1/0 is on line 3 (counting the blank). + source := "b = $23\n\na = 1 / 0\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + + eval := implDoc.NewEvaluator() + _ = eval.Evaluate(doc) // ErrPartialEvaluation expected + + var buf bytes.Buffer + formatter := &HTMLFormatter{} + opts := Options{Verbose: true} + + if err := formatter.Format(&buf, doc, opts); err != nil { + t.Fatalf("Format: %v", err) + } + + output := buf.String() + + // b = $23 should have a result + if !strings.Contains(output, "calc-inline-result") { + t.Error("expected b = $23 to have calc-inline-result") + } + + // a = 1 / 0 should have a diagnostic below the source line + if !strings.Contains(output, "calc-line-diagnostic") { + t.Errorf("expected a = 1 / 0 to have calc-line-diagnostic, but HTML output was:\n%s", output) + } + if !strings.Contains(output, "division by zero") { + t.Error("expected 'division by zero' in diagnostic") + } +} + +// TestHTMLFormatter_SemanticErrorShowsOnCorrectLine verifies that when a +// semantic error (like variable redefinition) aborts a block, the diagnostic +// appears on the correct line AND lines without per-line diagnostics still +// show the block-level error so no line appears silently blank. +func TestHTMLFormatter_SemanticErrorShowsOnCorrectLine(t *testing.T) { + source := "a = 1 / 0\na = 2\nc = 3\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + + eval := implDoc.NewEvaluator() + _ = eval.Evaluate(doc) // ErrPartialEvaluation expected + + var buf bytes.Buffer + formatter := &HTMLFormatter{} + opts := Options{Verbose: true} + + if err := formatter.Format(&buf, doc, opts); err != nil { + t.Fatalf("Format: %v", err) + } + + output := buf.String() + + // The redefinition error should appear as a per-line diagnostic (below line 2) + if !strings.Contains(output, "calc-line-diagnostic") { + t.Errorf("expected per-line diagnostic for redefinition, but HTML output was:\n%s", output) + } + if !strings.Contains(output, "immutable") { + t.Errorf("expected 'immutable' in diagnostic message, output:\n%s", output) + } + + // Block-level error div should NOT be shown when per-line diagnostics cover it + if strings.Contains(output, `
`) { + t.Error("block-level error div should be suppressed when per-line diagnostics exist") + } +} diff --git a/format/templates/calcmark.css b/format/templates/calcmark.css index 1c2b31ff..ff4d935d 100644 --- a/format/templates/calcmark.css +++ b/format/templates/calcmark.css @@ -66,6 +66,24 @@ body { margin-top: 0.5em; } +.calc-line-diagnostic { + font-size: 0.82em; + color: #d73a49; + padding: 0.15em 0 0.3em 1.5em; +} + +.calc-line-diagnostic::before { + content: "✗ "; +} + +.calc-line-diagnostic.calc-cascading { + color: #8b6914; +} + +.calc-line-diagnostic.calc-cascading::before { + content: "⚠ "; +} + .calc-warning { color: #b08800; background: #fff8e1; diff --git a/format/templates/default.gohtml b/format/templates/default.gohtml index e9e17ca3..934243f8 100644 --- a/format/templates/default.gohtml +++ b/format/templates/default.gohtml @@ -55,14 +55,17 @@ {{if eq .Type "calculation"}}
{{range $i, $line := .SourceLines}} -
+
{{$line.Source}} {{if $line.Result}} {{$line.Result}} {{end}}
+ {{- if $line.Error}} +
{{$line.Error}}
+ {{- end}} {{end}} - {{if .Error}} + {{if and .Error (not .HasLineDiagnostic)}}
Error: {{.Error}}
{{end}}
diff --git a/format/templates/preview.gohtml b/format/templates/preview.gohtml index 4bdadc49..e36410dd 100644 --- a/format/templates/preview.gohtml +++ b/format/templates/preview.gohtml @@ -43,14 +43,17 @@ {{if eq .Type "calculation"}}
{{range $i, $line := .SourceLines}} -
+
{{$line.Source}} {{if $line.Result}} {{$line.Result}} {{end}}
+ {{- if $line.Error}} +
{{$line.Error}}
+ {{- end}} {{end}} - {{if .Error}} + {{if and .Error (not .HasLineDiagnostic)}}
Error: {{.Error}}
{{end}}
diff --git a/impl/document/evaluator.go b/impl/document/evaluator.go index 5c828d7e..50a9be1a 100644 --- a/impl/document/evaluator.go +++ b/impl/document/evaluator.go @@ -1,6 +1,7 @@ package document import ( + "errors" "fmt" "slices" "strings" @@ -16,6 +17,16 @@ import ( "github.com/shopspring/decimal" ) +// ErrPartialEvaluation is returned when evaluation continued past errors. +// Callers can check errors.Is(err, ErrPartialEvaluation) to distinguish +// partial evaluation (some blocks failed) from total failure. +var ErrPartialEvaluation = errors.New("partial evaluation: one or more blocks had errors") + +// Diagnostic codes for error recovery. +const ( + diagCodeCascadingError = "cascading_error" +) + // NewDirectiveResolver creates an interpreter.DirectiveResolver that adapts // *document.Frontmatter for @directive resolution. Used by both the document // evaluator and the top-level Eval API. @@ -112,14 +123,15 @@ func (e *Evaluator) Evaluate(doc *document.Document) error { e.displayFormatter = &df } - // Evaluate blocks in document order (top-down) + // Evaluate blocks in document order (top-down). + // Continue past block errors to collect all diagnostics across the document. + var hasErrors bool for _, node := range doc.GetBlocks() { switch block := node.Block.(type) { case *document.CalcBlock: // Pass doc so @global/@exchange update frontmatter - err := e.evaluateCalcBlockWithDoc(node.ID, block, doc) - if err != nil { - return err + if err := e.evaluateCalcBlockWithDoc(node.ID, block, doc); err != nil { + hasErrors = true } case *document.TextBlock: // Check TextBlocks for lines that look like failed calculations @@ -133,6 +145,9 @@ func (e *Evaluator) Evaluate(doc *document.Document) error { // Resolve {{var}} tags in TextBlocks against the final environment interpolateTextBlocks(doc, e.env.GetAllVariables(), e.getDisplayFormatter()) + if hasErrors { + return ErrPartialEvaluation + } return nil } @@ -197,11 +212,14 @@ func (e *Evaluator) EvaluateBlock(doc *document.Document, blockID string) error return fmt.Errorf("block not found: %s", blockID) } - // PASS 1: Evaluate all blocks to collect final variable values + // PASS 1: Evaluate all blocks to collect final variable values. // This builds the environment with all variable assignments. // We track defined variables across all blocks to detect redefinitions. + // Continue past block errors to collect all diagnostics. e.env = interpreter.NewEnvironment() allDefinedVars := make(map[string]bool) + nonRecoverableBlocks := make(map[string]bool) // blocks with parse/redefinition errors (skip in pass 2) + var hasErrors bool for _, node := range doc.GetBlocks() { if cb, ok := node.Block.(*document.CalcBlock); ok { @@ -211,10 +229,27 @@ func (e *Evaluator) EvaluateBlock(doc *document.Document, blockID string) error if !strings.HasSuffix(source, "\n") { source += "\n" } - nodes, err := parser.Parse(source) - if err != nil { - cb.SetError(err) - return err + nodes, parseErr := parser.Parse(source) + if parseErr != nil { + cb.SetError(parseErr) + // Add diagnostic but continue to next block + if pe, ok := parseErr.(*parser.ParseError); ok && pe.Line > 0 { + var lineOff int + if doc != nil { + lineOff = blockLineOffset(doc, node.ID) + } + cb.AddDiagnostic(document.Diagnostic{ + Severity: "error", + Code: "parse_error", + Message: pe.Message, + Line: pe.Line, + Column: pe.Column, + DocLine: pe.Line + lineOff, + }) + } + hasErrors = true + nonRecoverableBlocks[node.ID] = true + continue } // Extract variables that THIS execution will define @@ -233,6 +268,7 @@ func (e *Evaluator) EvaluateBlock(doc *document.Document, blockID string) error hasBeenEvaluated := len(cb.Results()) > 0 && !cb.IsDirty() previouslyDefined := cb.Variables() + blockHasRedefError := false for _, varName := range currentlyDefining { // Skip if this block previously defined this variable if hasBeenEvaluated && slices.Contains(previouslyDefined, varName) { @@ -240,22 +276,45 @@ func (e *Evaluator) EvaluateBlock(doc *document.Document, blockID string) error } // Check for conflict with other blocks if allDefinedVars[varName] { - err := fmt.Errorf("variable_redefinition: variable '%s' is already defined", varName) - cb.SetError(err) - return err + redefErr := fmt.Errorf("variable_redefinition: variable '%s' is already defined", varName) + cb.SetError(redefErr) + cb.AddDiagnostic(document.Diagnostic{ + Severity: "error", + Code: "variable_redefinition", + Message: redefErr.Error(), + }) + // Mark only the conflicting variable as errored; + // non-conflicting variables in this block will be evaluated + // normally if the block proceeds (or marked errored individually + // if evaluation fails for other reasons). + e.env.SetError(varName, redefErr) + hasErrors = true + blockHasRedefError = true + break + } + } + if blockHasRedefError { + nonRecoverableBlocks[node.ID] = true + // Still track variables as defined for downstream redefinition checks + for _, varName := range currentlyDefining { + allDefinedVars[varName] = true } + continue } // Now evaluate the block - err = e.evaluateCalcBlockWithDoc(node.ID, cb, doc) - if err != nil { - return err + if err := e.evaluateCalcBlockWithDoc(node.ID, cb, doc); err != nil { + hasErrors = true } - // Mark variables as defined + // Mark variables as defined (includes errored vars from evaluation) for _, varName := range cb.Variables() { allDefinedVars[varName] = true } + // Also mark variables extracted from AST (for blocks that partially failed) + for _, varName := range currentlyDefining { + allDefinedVars[varName] = true + } } } @@ -274,16 +333,23 @@ func (e *Evaluator) EvaluateBlock(doc *document.Document, blockID string) error } } - // PASS 2: Re-evaluate each block using the final variable values - // Only allow a block to SET a variable if it's the last definition - // This ensures earlier definitions (like a=10) don't overwrite later ones (a=20) + // PASS 2: Re-evaluate each block using the final variable values. + // Only allow a block to SET a variable if it's the last definition. + // This ensures earlier definitions (like a=10) don't overwrite later ones (a=20). + // Continue past block errors to collect all diagnostics. reactiveEnv := finalEnv.Clone() for _, node := range doc.GetBlocks() { if cb, ok := node.Block.(*document.CalcBlock); ok { - err := e.evaluateCalcBlockSelective(node.ID, cb, reactiveEnv, lastDefBlock, doc) - if err != nil { - return err + // Skip blocks with non-recoverable errors from pass 1 (parse/redefinition). + // These cannot succeed in pass 2 regardless of the environment state. + // Runtime errors (e.g., forward references that failed in pass 1) should be + // retried in pass 2 since the reactive env now has all variable values. + if nonRecoverableBlocks[node.ID] { + continue + } + if err := e.evaluateCalcBlockSelective(node.ID, cb, reactiveEnv, lastDefBlock, doc); err != nil { + hasErrors = true } } } @@ -297,6 +363,9 @@ func (e *Evaluator) EvaluateBlock(doc *document.Document, blockID string) error // Resolve {{var}} tags in TextBlocks against the final environment interpolateTextBlocks(doc, e.env.GetAllVariables(), e.getDisplayFormatter()) + if hasErrors { + return ErrPartialEvaluation + } return nil } @@ -312,6 +381,7 @@ func (e *Evaluator) EvaluateBlock(doc *document.Document, blockID string) error // The blocks should be in dependency order (use GetBlocksInDependencyOrder). // The environment is NOT reset - it maintains accumulated state from previous evaluations. func (e *Evaluator) EvaluateAffectedBlocks(doc *document.Document, blockIDs []string) error { + var hasErrors bool for _, blockID := range blockIDs { node, ok := doc.GetBlock(blockID) if !ok { @@ -319,9 +389,8 @@ func (e *Evaluator) EvaluateAffectedBlocks(doc *document.Document, blockIDs []st } if cb, ok := node.Block.(*document.CalcBlock); ok { - err := e.evaluateCalcBlock(blockID, cb) - if err != nil { - return err + if err := e.evaluateCalcBlock(blockID, cb); err != nil { + hasErrors = true } } } @@ -333,6 +402,9 @@ func (e *Evaluator) EvaluateAffectedBlocks(doc *document.Document, blockIDs []st // Re-run interpolation on all TextBlocks — variable values may have changed. interpolateTextBlocks(doc, e.env.GetAllVariables(), e.getDisplayFormatter()) + if hasErrors { + return ErrPartialEvaluation + } return nil } @@ -390,6 +462,14 @@ func (e *Evaluator) evaluateCalcBlockSelective(blockID string, block *document.C checker.GetEnvironment().Set(varName, value) } } + // Also register errored variables so the semantic checker treats them as "defined". + for varName := range env.GetAllErroredVars() { + if !slices.Contains(previouslyDefinedVars, varName) { + if _, alreadySet := checker.GetEnvironment().Get(varName); !alreadySet { + checker.GetEnvironment().Set(varName, nil) + } + } + } // Provide frontmatter context for @directive validation if doc != nil { @@ -398,9 +478,12 @@ func (e *Evaluator) evaluateCalcBlockSelective(blockID string, block *document.C } diagnostics := checker.Check(nodes) + + // Collect ALL semantic errors (not just the first) so every + // redefinition or type error in the block gets a diagnostic. + var firstSemanticErr error for _, diag := range diagnostics { if diag.Severity == semantic.Error { - // Store structured diagnostic with position info blockDiag := document.Diagnostic{ Severity: "error", Code: diag.Code, @@ -416,19 +499,28 @@ func (e *Evaluator) evaluateCalcBlockSelective(blockID string, block *document.C } block.AddDiagnostic(blockDiag) - // Also set legacy error for backwards compatibility - if blockDiag.DocLine > 0 { - err = fmt.Errorf("line %d: %s: %s", blockDiag.DocLine, diag.Code, diag.Message) - } else { - err = fmt.Errorf("%s: %s", diag.Code, diag.Message) + if firstSemanticErr == nil { + if blockDiag.DocLine > 0 { + firstSemanticErr = fmt.Errorf("line %d: %s: %s", blockDiag.DocLine, diag.Code, diag.Message) + } else { + firstSemanticErr = fmt.Errorf("%s: %s", diag.Code, diag.Message) + } + } + } + } + if firstSemanticErr != nil { + block.SetError(firstSemanticErr) + // Mark all variables defined in this block as errored (O(n) once) + for _, astNode := range nodes { + if assign, ok := astNode.(*ast.Assignment); ok { + env.SetError(assign.Name, firstSemanticErr) } - block.SetError(err) - return err } + return firstSemanticErr } - // 3. Interpret with a COPY of the environment - // We'll selectively copy back only authoritative assignments + // 3. Interpret with a COPY of the environment, statement by statement. + // Failed statements get nil placeholders to maintain 1:1 alignment with nodes. evalEnv := env.Clone() interp := interpreter.NewInterpreterWithEnv(evalEnv) if doc != nil && doc.GetFrontmatter() != nil { @@ -439,28 +531,85 @@ func (e *Evaluator) evaluateCalcBlockSelective(blockID string, block *document.C interp.SetMeasurement(&fm.Measurement.MeasurementConfig) } } - results, err := interp.Eval(nodes) - if err != nil { - block.SetError(err) - return err + results := make([]types.Type, 0, len(nodes)) + var firstErr error + hadErrors := false + + for _, node := range nodes { + nodeResults, evalErr := interp.Eval([]ast.Node{node}) + if evalErr != nil { + results = append(results, nil) + hadErrors = true + if firstErr == nil { + firstErr = evalErr + } + + // Create diagnostic based on error type + var cascErr *interpreter.CascadingError + diag := document.Diagnostic{ + Message: evalErr.Error(), + } + if errors.As(evalErr, &cascErr) { + diag.Code = diagCodeCascadingError + diag.Severity = "hint" + diag.Detailed = cascErr.VarName + } else { + diag.Code = "eval_error" + diag.Severity = "error" + } + if r := node.GetRange(); r != nil && r.Start.Line > 0 { + diag.Line = r.Start.Line + diag.Column = r.Start.Column + diag.DocLine = r.Start.Line + lineOff + } + block.AddDiagnostic(diag) + + // Mark assigned variable as errored in the cloned env + if assign, ok := node.(*ast.Assignment); ok { + evalEnv.SetError(assign.Name, evalErr) + } + continue + } + if len(nodeResults) > 0 { + results = append(results, nodeResults[0]) + } else { + results = append(results, nil) // placeholder for zero-result statements (e.g., directives) + } } - // 4. Store results + // 4. Store results (may contain nil entries for failed statements) block.SetResults(results) - if len(results) > 0 { - block.SetLastValue(results[len(results)-1]) + + // SetLastValue with the last non-nil result + for i := len(results) - 1; i >= 0; i-- { + if results[i] != nil { + block.SetLastValue(results[i]) + break + } } - // 5. Only update env for variables where this block is the last definition - // This prevents earlier blocks (a=10) from overwriting later ones (a=20) + // 5. Only update env for variables where this block is the last definition. + // Propagate errors back to the shared environment so downstream blocks see them. for _, varName := range block.Variables() { if lastDefBlock[varName] == blockID { - if val, ok := evalEnv.Get(varName); ok { + if evalErrForVar, hasErr := evalEnv.GetError(varName); hasErr { + // Variable errored in this block — propagate error to shared env + env.SetError(varName, evalErrForVar) + } else if val, ok := evalEnv.Get(varName); ok { env.Set(varName, val) + // Clear any previous error for this variable (it succeeded now) + env.ClearError(varName) } } } + if hadErrors { + if firstErr != nil { + block.SetError(firstErr) + } + return firstErr + } + block.SetDirty(false) return nil } @@ -528,6 +677,16 @@ func (e *Evaluator) evaluateCalcBlockWithDoc(blockID string, block *document.Cal } checker.GetEnvironment().Set(varName, value) } + // Also register errored variables so the semantic checker treats them as "defined". + // The interpreter will detect them as errored and produce CascadingError. + for varName := range e.env.GetAllErroredVars() { + if hasBeenEvaluated && slices.Contains(previouslyDefinedVars, varName) { + continue + } + if _, alreadySet := checker.GetEnvironment().Get(varName); !alreadySet { + checker.GetEnvironment().Set(varName, nil) + } + } // Provide frontmatter context for @directive validation if doc != nil { @@ -537,10 +696,10 @@ func (e *Evaluator) evaluateCalcBlockWithDoc(blockID string, block *document.Cal diagnostics := checker.Check(nodes) - // Check for errors + // Collect ALL semantic errors (not just the first). + var firstSemanticErr error for _, diag := range diagnostics { if diag.Severity == semantic.Error { - // Store structured diagnostic with position info blockDiag := document.Diagnostic{ Severity: "error", Code: diag.Code, @@ -556,19 +715,28 @@ func (e *Evaluator) evaluateCalcBlockWithDoc(blockID string, block *document.Cal } block.AddDiagnostic(blockDiag) - // Also set legacy error for backwards compatibility - if blockDiag.DocLine > 0 { - err = fmt.Errorf("line %d: %s: %s", blockDiag.DocLine, diag.Code, diag.Message) - } else { - err = fmt.Errorf("%s: %s", diag.Code, diag.Message) + if firstSemanticErr == nil { + if blockDiag.DocLine > 0 { + firstSemanticErr = fmt.Errorf("line %d: %s: %s", blockDiag.DocLine, diag.Code, diag.Message) + } else { + firstSemanticErr = fmt.Errorf("%s: %s", diag.Code, diag.Message) + } } - block.SetError(err) - return err } } + if firstSemanticErr != nil { + block.SetError(firstSemanticErr) + for _, astNode := range nodes { + if assign, ok := astNode.(*ast.Assignment); ok { + e.env.SetError(assign.Name, firstSemanticErr) + } + } + return firstSemanticErr + } // 3. Interpret statements with shared environment - // Evaluate statements one by one to collect partial results even if a later statement fails + // Evaluate statements one by one to collect partial results even if a later statement fails. + // Failed statements get nil placeholders to maintain 1:1 alignment with nodes. interp := interpreter.NewInterpreterWithEnv(e.env) if doc != nil && doc.GetFrontmatter() != nil { fm := doc.GetFrontmatter() @@ -579,37 +747,35 @@ func (e *Evaluator) evaluateCalcBlockWithDoc(blockID string, block *document.Cal } } results := make([]types.Type, 0, len(nodes)) - var evalErr error - var failingNodeIdx = -1 + var firstErr error + hadErrors := false - for i, node := range nodes { + for _, node := range nodes { nodeResults, err := interp.Eval([]ast.Node{node}) if err != nil { - evalErr = err - failingNodeIdx = i - break - } - if len(nodeResults) > 0 { - results = append(results, nodeResults[0]) - } - } + // Append nil placeholder to maintain 1:1 alignment with nodes + results = append(results, nil) + hadErrors = true - // Store partial results even if there was an error - if len(results) > 0 { - block.SetResults(results) - block.SetLastValue(results[len(results)-1]) - } + // Track first error for legacy block.SetError() compatibility + if firstErr == nil { + firstErr = err + } - // If there was an error, create diagnostic and return - if evalErr != nil { - // Create diagnostic with line number for the failing node - if failingNodeIdx >= 0 && failingNodeIdx < len(nodes) { - node := nodes[failingNodeIdx] + // Create diagnostic based on error type + var cascErr *interpreter.CascadingError diag := document.Diagnostic{ - Severity: "error", - Code: "eval_error", - Message: evalErr.Error(), + Message: err.Error(), } + if errors.As(err, &cascErr) { + diag.Code = diagCodeCascadingError + diag.Severity = "hint" + diag.Detailed = cascErr.VarName + } else { + diag.Code = "eval_error" + diag.Severity = "error" + } + // Use node's Range if available to get line number. // All AST nodes implement GetRange() via the Node interface. // Guard against zero-valued ranges (some nodes use &ast.Range{}). @@ -620,17 +786,41 @@ func (e *Evaluator) evaluateCalcBlockWithDoc(blockID string, block *document.Cal } block.AddDiagnostic(diag) - // Include document-absolute line number in returned error - if diag.DocLine > 0 { - evalErr = fmt.Errorf("line %d: %w", diag.DocLine, evalErr) + // If the statement is an assignment, mark the variable as errored + if assign, ok := node.(*ast.Assignment); ok { + e.env.SetError(assign.Name, err) } + + continue + } + if len(nodeResults) > 0 { + results = append(results, nodeResults[0]) + } else { + results = append(results, nil) // placeholder for zero-result statements (e.g., directives) } + } + + // Store results (may contain nil entries for failed statements) + block.SetResults(results) - block.SetError(evalErr) - return evalErr + // SetLastValue with the last non-nil result + for i := len(results) - 1; i >= 0; i-- { + if results[i] != nil { + block.SetLastValue(results[i]) + break + } + } + + // If there were errors, set legacy error and return first error + if hadErrors { + // Include document-absolute line number in returned error + if firstErr != nil { + block.SetError(firstErr) + } + return firstErr } - // Mark as clean (evaluated successfully) + // Mark as clean (evaluated successfully — all statements passed) block.SetDirty(false) return nil diff --git a/impl/document/evaluator_benchmark_test.go b/impl/document/evaluator_benchmark_test.go new file mode 100644 index 00000000..ed899910 --- /dev/null +++ b/impl/document/evaluator_benchmark_test.go @@ -0,0 +1,61 @@ +package document + +import ( + "fmt" + "strings" + "testing" + + "github.com/CalcMark/go-calcmark/spec/document" +) + +// BenchmarkEvaluate_ValidDocument benchmarks full document evaluation +// with a realistic multi-statement document (no errors). +// This is the hot path — error recovery must not regress it. +func BenchmarkEvaluate_ValidDocument(b *testing.B) { + // Build a 100-statement document: x1 = 1, x2 = x1 + 1, ..., x100 = x99 + 1 + var lines []string + lines = append(lines, "x1 = 1") + for i := 2; i <= 100; i++ { + lines = append(lines, fmt.Sprintf("x%d = x%d + 1", i, i-1)) + } + source := strings.Join(lines, "\n") + "\n" + + doc, err := document.NewDocument(source) + if err != nil { + b.Fatalf("NewDocument: %v", err) + } + + b.ResetTimer() + for b.Loop() { + eval := NewEvaluator() + if err := eval.Evaluate(doc); err != nil { + b.Fatalf("Evaluate: %v", err) + } + } +} + +// BenchmarkEvaluate_WithErrors benchmarks error recovery overhead: +// a document where half the statements fail. +func BenchmarkEvaluate_WithErrors(b *testing.B) { + // 50 successful + 50 errored (cascading from first div-by-zero) + var lines []string + lines = append(lines, "bad = 1 / 0") + for i := 1; i <= 49; i++ { + lines = append(lines, fmt.Sprintf("ok%d = %d", i, i)) + } + for i := 1; i <= 50; i++ { + lines = append(lines, fmt.Sprintf("fail%d = bad + %d", i, i)) + } + source := strings.Join(lines, "\n") + "\n" + + doc, err := document.NewDocument(source) + if err != nil { + b.Fatalf("NewDocument: %v", err) + } + + b.ResetTimer() + for b.Loop() { + eval := NewEvaluator() + eval.Evaluate(doc) // ErrPartialEvaluation expected + } +} diff --git a/impl/document/evaluator_test.go b/impl/document/evaluator_test.go index 94cf30d5..caa10f1b 100644 --- a/impl/document/evaluator_test.go +++ b/impl/document/evaluator_test.go @@ -1,9 +1,11 @@ package document import ( + "errors" "strings" "testing" + "github.com/CalcMark/go-calcmark/impl/interpreter" "github.com/CalcMark/go-calcmark/spec/document" ) @@ -329,8 +331,16 @@ func TestGlobalScopeEnvironmentPersistence(t *testing.T) { if err == nil { t.Fatal("Expected error for variable redefinition, but got nil") } - if !strings.Contains(err.Error(), "already defined") && !strings.Contains(err.Error(), "redefinition") { - t.Errorf("Expected redefinition error, got: %v", err) + if !errors.Is(err, ErrPartialEvaluation) { + t.Errorf("Expected ErrPartialEvaluation, got: %v", err) + } + // The redefinition error should be on the block's diagnostics + redefNode, _ := doc.GetBlock(result.ModifiedBlockID) + redefBlock := redefNode.Block.(*document.CalcBlock) + if redefBlock.Error() == nil { + t.Error("Expected block to have error for variable redefinition") + } else if !strings.Contains(redefBlock.Error().Error(), "already defined") && !strings.Contains(redefBlock.Error().Error(), "redefinition") { + t.Errorf("Expected redefinition error on block, got: %v", redefBlock.Error()) } } @@ -350,14 +360,25 @@ z = x + y t.Fatalf("Failed to create document: %v", err) } - // Evaluation should fail + // Evaluation should return ErrPartialEvaluation (continues past errors) eval := NewEvaluator() err = eval.Evaluate(doc) if err == nil { t.Fatal("Expected error for variable redefinition within single block, got nil") } - if !strings.Contains(err.Error(), "already defined") && !strings.Contains(err.Error(), "redefinition") { - t.Errorf("Expected redefinition error, got: %v", err) + if !errors.Is(err, ErrPartialEvaluation) { + t.Errorf("Expected ErrPartialEvaluation, got: %v", err) + } + // The redefinition error should be on the block's error + blocks := doc.GetBlocks() + if len(blocks) > 0 { + if cb, ok := blocks[0].Block.(*document.CalcBlock); ok { + if cb.Error() == nil { + t.Error("Expected block to have redefinition error") + } else if !strings.Contains(cb.Error().Error(), "already defined") && !strings.Contains(cb.Error().Error(), "redefinition") { + t.Errorf("Expected redefinition error on block, got: %v", cb.Error()) + } + } } } @@ -572,7 +593,7 @@ func TestEvaluateForwardReference(t *testing.T) { } interp := tb.InterpolatedSource() for _, line := range interp { - if strings.Contains(line, "**42**") { + if strings.Contains(line, "42") && !strings.Contains(line, "{{") { return // Success — forward reference resolved } } @@ -607,3 +628,1132 @@ func TestEvaluateInterpolationPreservesSource(t *testing.T) { } t.Error("Source() should still contain raw {{x}} tag") } + +// --- Statement-level error recovery tests (Unit 3) --- + +// TestErrorRecovery_DivByZeroThenSuccess tests that a block continues past +// a division-by-zero error and evaluates subsequent independent statements. +func TestErrorRecovery_DivByZeroThenSuccess(t *testing.T) { + // a = 1/0 fails, b = 10 should succeed + source := "a = 1 / 0\nb = 10\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + // Should return an error (first error) + if err == nil { + t.Fatal("Expected error from division by zero, got nil") + } + + blocks := doc.GetBlocks() + if len(blocks) == 0 { + t.Fatal("Expected at least 1 block") + } + + cb := blocks[0].Block.(*document.CalcBlock) + + // Results must have exactly len(statements) entries + results := cb.Results() + stmts := cb.Statements() + if len(results) != len(stmts) { + t.Fatalf("len(results)=%d != len(statements)=%d", len(results), len(stmts)) + } + if len(results) != 2 { + t.Fatalf("Expected 2 results, got %d", len(results)) + } + + // First result is nil (failed) + if results[0] != nil { + t.Errorf("Expected nil for failed statement, got %v", results[0]) + } + + // Second result is 10 (succeeded) + if results[1] == nil { + t.Fatal("Expected non-nil result for b = 10") + } + if results[1].String() != "10" { + t.Errorf("Expected b = 10, got %q", results[1].String()) + } + + // Block should have error set (first error) and stay dirty + if cb.Error() == nil { + t.Error("Expected block.Error() to be non-nil") + } + if !cb.IsDirty() { + t.Error("Expected block to stay dirty when statements have errors") + } + + // Check diagnostics + diags := cb.Diagnostics() + if len(diags) == 0 { + t.Fatal("Expected at least 1 diagnostic") + } + found := false + for _, d := range diags { + if d.Code == "eval_error" && d.Severity == "error" { + found = true + break + } + } + if !found { + t.Error("Expected eval_error diagnostic with error severity") + } + + // b should be in the environment + env := eval.GetEnvironment() + if val, ok := env.Get("b"); !ok || val.String() != "10" { + t.Errorf("Expected b=10 in environment, got %v (ok=%v)", val, ok) + } + + // a should be marked as errored in the environment + if _, errored := env.GetError("a"); !errored { + t.Error("Expected variable 'a' to be marked as errored") + } +} + +// TestErrorRecovery_CascadingError tests that referencing an errored variable +// produces a cascading_error diagnostic with hint severity. +func TestErrorRecovery_CascadingError(t *testing.T) { + // a = 1/0 fails, c = a * 2 references errored "a" -> cascading error + source := "a = 1 / 0\nc = a * 2\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if err == nil { + t.Fatal("Expected error, got nil") + } + + blocks := doc.GetBlocks() + cb := blocks[0].Block.(*document.CalcBlock) + + results := cb.Results() + stmts := cb.Statements() + if len(results) != len(stmts) { + t.Fatalf("len(results)=%d != len(statements)=%d", len(results), len(stmts)) + } + + // Both results should be nil (both failed) + if results[0] != nil { + t.Errorf("Expected nil for a = 1/0, got %v", results[0]) + } + if results[1] != nil { + t.Errorf("Expected nil for c = a*2, got %v", results[1]) + } + + // Check diagnostics + diags := cb.Diagnostics() + if len(diags) < 2 { + t.Fatalf("Expected at least 2 diagnostics, got %d", len(diags)) + } + + // First diagnostic should be eval_error for a = 1/0 + foundEvalError := false + foundCascading := false + for _, d := range diags { + if d.Code == "eval_error" && d.Severity == "error" { + foundEvalError = true + } + if d.Code == diagCodeCascadingError && d.Severity == "hint" { + foundCascading = true + if d.Detailed != "a" { + t.Errorf("Expected cascading_error Detailed='a', got %q", d.Detailed) + } + } + } + if !foundEvalError { + t.Error("Expected eval_error diagnostic") + } + if !foundCascading { + t.Error("Expected cascading_error diagnostic with hint severity") + } + + // Both a and c should be errored + env := eval.GetEnvironment() + if _, errored := env.GetError("a"); !errored { + t.Error("Expected 'a' to be errored") + } + if _, errored := env.GetError("c"); !errored { + t.Error("Expected 'c' to be errored") + } + + // The error for c should be a CascadingError + cErr, _ := env.GetError("c") + var cascErr *interpreter.CascadingError + if !errors.As(cErr, &cascErr) { + t.Errorf("Expected CascadingError for 'c', got %T: %v", cErr, cErr) + } else if cascErr.VarName != "a" { + t.Errorf("Expected cascading error VarName='a', got %q", cascErr.VarName) + } +} + +// TestErrorRecovery_AllSuccess tests that a block with all successful statements +// behaves exactly as before (no regressions). +func TestErrorRecovery_AllSuccess(t *testing.T) { + source := "a = 5\nb = a + 10\nc = b * 2\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if err != nil { + t.Fatalf("Expected no error, got: %v", err) + } + + blocks := doc.GetBlocks() + cb := blocks[0].Block.(*document.CalcBlock) + + results := cb.Results() + stmts := cb.Statements() + if len(results) != len(stmts) { + t.Fatalf("len(results)=%d != len(statements)=%d", len(results), len(stmts)) + } + if len(results) != 3 { + t.Fatalf("Expected 3 results, got %d", len(results)) + } + + // Verify values + expected := []string{"5", "15", "30"} + for i, exp := range expected { + if results[i] == nil { + t.Errorf("Result %d is nil, expected %s", i, exp) + continue + } + if results[i].String() != exp { + t.Errorf("Result %d: expected %s, got %s", i, exp, results[i].String()) + } + } + + // Block should be clean + if cb.IsDirty() { + t.Error("Expected block to be clean after successful evaluation") + } + if cb.Error() != nil { + t.Errorf("Expected no error, got: %v", cb.Error()) + } + if len(cb.Diagnostics()) != 0 { + t.Errorf("Expected no diagnostics, got %d", len(cb.Diagnostics())) + } +} + +// TestErrorRecovery_AllStatementsFail tests a block where every statement fails. +func TestErrorRecovery_AllStatementsFail(t *testing.T) { + // a = 1/0, b = 1/0 — both fail independently + source := "a = 1 / 0\nb = 1 / 0\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if err == nil { + t.Fatal("Expected error, got nil") + } + + blocks := doc.GetBlocks() + cb := blocks[0].Block.(*document.CalcBlock) + + results := cb.Results() + stmts := cb.Statements() + if len(results) != len(stmts) { + t.Fatalf("len(results)=%d != len(statements)=%d", len(results), len(stmts)) + } + + // All results should be nil + for i, r := range results { + if r != nil { + t.Errorf("Expected nil for result %d, got %v", i, r) + } + } + + // Block should stay dirty + if !cb.IsDirty() { + t.Error("Expected block to stay dirty when all statements fail") + } + + // LastValue should be nil (no successful results) + if cb.LastValue() != nil { + t.Errorf("Expected nil LastValue, got %v", cb.LastValue()) + } + + // Should have 2 diagnostics + diags := cb.Diagnostics() + if len(diags) != 2 { + t.Fatalf("Expected 2 diagnostics, got %d", len(diags)) + } +} + +// TestErrorRecovery_MultipleErroredVarsCascade tests that a statement +// referencing multiple errored variables produces one cascading error. +func TestErrorRecovery_MultipleErroredVarsCascade(t *testing.T) { + // a = 1/0, b = 1/0, c = a + b — c depends on two errored vars + source := "a = 1 / 0\nb = 1 / 0\nc = a + b\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if err == nil { + t.Fatal("Expected error, got nil") + } + + blocks := doc.GetBlocks() + cb := blocks[0].Block.(*document.CalcBlock) + + results := cb.Results() + if len(results) != 3 { + t.Fatalf("Expected 3 results, got %d", len(results)) + } + + // All nil + for i, r := range results { + if r != nil { + t.Errorf("Expected nil for result %d, got %v", i, r) + } + } + + // c should have a cascading_error diagnostic + diags := cb.Diagnostics() + foundCascading := false + for _, d := range diags { + if d.Code == diagCodeCascadingError { + foundCascading = true + // Should name one of the root-cause variables + if d.Detailed != "a" && d.Detailed != "b" { + t.Errorf("Expected Detailed to be 'a' or 'b', got %q", d.Detailed) + } + } + } + if !foundCascading { + t.Error("Expected cascading_error diagnostic for c = a + b") + } +} + +// TestErrorRecovery_SuccessThenFailInSameBlock tests that a successful +// statement's value persists in the environment even when a later statement fails. +func TestErrorRecovery_SuccessThenFailInSameBlock(t *testing.T) { + source := "y = 15\nz = 1 / 0\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if err == nil { + t.Fatal("Expected error from z = 1/0, got nil") + } + + // y should be in the environment (succeeded) + env := eval.GetEnvironment() + if val, ok := env.Get("y"); !ok || val.String() != "15" { + t.Errorf("Expected y=15 in environment, got %v (ok=%v)", val, ok) + } + + // z should be errored + if _, errored := env.GetError("z"); !errored { + t.Error("Expected 'z' to be errored") + } +} + +// TestErrorRecovery_DiagnosticLineNumbers tests that diagnostics have +// correct Line and DocLine values. +func TestErrorRecovery_DiagnosticLineNumbers(t *testing.T) { + // Two blocks: first succeeds, second has error on its 2nd statement + source := "x = 10\n\n\ny = 5\nz = 1 / 0\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if err == nil { + t.Fatal("Expected error, got nil") + } + + // Find the block with the error + for _, node := range doc.GetBlocks() { + cb, ok := node.Block.(*document.CalcBlock) + if !ok || cb.Error() == nil { + continue + } + + diags := cb.Diagnostics() + if len(diags) == 0 { + t.Fatal("Expected diagnostics on errored block") + } + + // The error diagnostic should have line info + for _, d := range diags { + if d.Code == "eval_error" { + if d.Line == 0 { + t.Error("Expected non-zero Line on eval_error diagnostic") + } + // DocLine should be greater than Line (block-relative) due to preceding blocks + if d.DocLine == 0 { + t.Error("Expected non-zero DocLine on eval_error diagnostic") + } + return + } + } + t.Error("Expected eval_error diagnostic in errored block") + } +} + +// TestErrorRecovery_ResultsSliceInvariant verifies that +// len(block.Results()) == len(block.Statements()) always holds. +func TestErrorRecovery_ResultsSliceInvariant(t *testing.T) { + tests := []struct { + name string + source string + }{ + {"all success", "a = 1\nb = 2\nc = 3\n"}, + {"first fails", "a = 1 / 0\nb = 10\n"}, + {"last fails", "a = 10\nb = 1 / 0\n"}, + {"middle fails", "a = 10\nb = 1 / 0\nc = 20\n"}, + {"all fail", "a = 1 / 0\nb = 1 / 0\n"}, + {"cascading", "a = 1 / 0\nb = a + 1\n"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + doc, err := document.NewDocument(tt.source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + _ = eval.Evaluate(doc) // may or may not error + + for _, node := range doc.GetBlocks() { + cb, ok := node.Block.(*document.CalcBlock) + if !ok { + continue + } + results := cb.Results() + stmts := cb.Statements() + if len(results) != len(stmts) { + t.Errorf("Invariant violated: len(results)=%d != len(statements)=%d", + len(results), len(stmts)) + } + } + }) + } +} + +// TestErrorRecovery_BlockWithErrorsIsDirty verifies that blocks with +// any error stay dirty and have block.Error() set. +func TestErrorRecovery_BlockWithErrorsIsDirty(t *testing.T) { + source := "a = 10\nb = 1 / 0\nc = 20\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + _ = eval.Evaluate(doc) + + blocks := doc.GetBlocks() + cb := blocks[0].Block.(*document.CalcBlock) + + if cb.Error() == nil { + t.Error("Expected block.Error() to be non-nil") + } + if !cb.IsDirty() { + t.Error("Expected block.IsDirty() == true") + } + + // But successful results should still be present + results := cb.Results() + if results[0] == nil || results[0].String() != "10" { + t.Errorf("Expected a=10, got %v", results[0]) + } + if results[1] != nil { + t.Errorf("Expected nil for b=1/0, got %v", results[1]) + } + if results[2] == nil || results[2].String() != "20" { + t.Errorf("Expected c=20, got %v", results[2]) + } +} + +// --- Block-level error recovery tests (Unit 4) --- + +// TestBlockRecovery_EvaluateContinuesPastErrorBlock tests that Evaluate() +// continues evaluating blocks after a block has errors, and returns ErrPartialEvaluation. +func TestBlockRecovery_EvaluateContinuesPastErrorBlock(t *testing.T) { + // Block 1: has error (undefined var) + // Block 2: independent, should succeed + // Block 3: independent, should succeed + source := "a = unknown_var + 1\n\n\nb = 10\n\n\nc = 20\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + + // Should return ErrPartialEvaluation, not nil + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation, got: %v", err) + } + + // Verify block 1 has error + blocks := doc.GetBlocks() + calcBlocks := make([]*document.CalcBlock, 0) + for _, node := range blocks { + if cb, ok := node.Block.(*document.CalcBlock); ok { + calcBlocks = append(calcBlocks, cb) + } + } + if len(calcBlocks) != 3 { + t.Fatalf("Expected 3 calc blocks, got %d", len(calcBlocks)) + } + + if calcBlocks[0].Error() == nil { + t.Error("Block 1 should have an error") + } + + // Block 2 should succeed + if calcBlocks[1].Error() != nil { + t.Errorf("Block 2 should not have error, got: %v", calcBlocks[1].Error()) + } + if calcBlocks[1].LastValue() == nil || calcBlocks[1].LastValue().String() != "10" { + t.Errorf("Block 2: expected b=10, got %v", calcBlocks[1].LastValue()) + } + + // Block 3 should succeed + if calcBlocks[2].Error() != nil { + t.Errorf("Block 3 should not have error, got: %v", calcBlocks[2].Error()) + } + if calcBlocks[2].LastValue() == nil || calcBlocks[2].LastValue().String() != "20" { + t.Errorf("Block 3: expected c=20, got %v", calcBlocks[2].LastValue()) + } +} + +// TestBlockRecovery_NoErrorsReturnsNil tests that Evaluate() returns nil when all blocks succeed. +func TestBlockRecovery_NoErrorsReturnsNil(t *testing.T) { + source := "a = 10\n\n\nb = a + 5\n\n\nc = b * 2\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if err != nil { + t.Fatalf("Expected nil error for valid document, got: %v", err) + } +} + +// TestBlockRecovery_CascadingAcrossBlocks tests that a block referencing an errored +// variable from a prior block gets cascading_error diagnostics. +func TestBlockRecovery_CascadingAcrossBlocks(t *testing.T) { + // Block 1: a = 1/0 (div by zero error) + // Block 2: b = a * 2 (cascading error) + source := "a = 1 / 0\n\n\nb = a * 2\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation, got: %v", err) + } + + // Block 1 should have eval_error + blocks := doc.GetBlocks() + calcBlocks := make([]*document.CalcBlock, 0) + for _, node := range blocks { + if cb, ok := node.Block.(*document.CalcBlock); ok { + calcBlocks = append(calcBlocks, cb) + } + } + if len(calcBlocks) != 2 { + t.Fatalf("Expected 2 calc blocks, got %d", len(calcBlocks)) + } + + if calcBlocks[0].Error() == nil { + t.Error("Block 1 should have error") + } + + // Block 2 should also have an error (cascading) + if calcBlocks[1].Error() == nil { + t.Error("Block 2 should have cascading error") + } + + // Block 2's diagnostic should be cascading_error with hint severity + diags := calcBlocks[1].Diagnostics() + foundCascading := false + for _, d := range diags { + if d.Code == "cascading_error" && d.Severity == "hint" { + foundCascading = true + } + } + if !foundCascading { + t.Errorf("Expected cascading_error hint diagnostic on block 2, got: %v", diags) + } +} + +// TestBlockRecovery_AllBlocksHaveErrors tests that ErrPartialEvaluation is returned +// and all blocks have diagnostics when every block fails. +func TestBlockRecovery_AllBlocksHaveErrors(t *testing.T) { + source := "a = unknown1\n\n\nb = unknown2\n\n\nc = unknown3\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation, got: %v", err) + } + + // Every block should have errors + for _, node := range doc.GetBlocks() { + if cb, ok := node.Block.(*document.CalcBlock); ok { + if cb.Error() == nil { + t.Errorf("Block %s should have error", node.ID[:8]) + } + } + } +} + +// TestBlockRecovery_EvaluateBlockPass1Recovery tests that EvaluateBlock pass 1 +// continues past parse/eval errors in one block to evaluate other blocks. +func TestBlockRecovery_EvaluateBlockPass1Recovery(t *testing.T) { + source := "a = 1 / 0\n\n\nb = 10\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + // Initial evaluation to set things up + err = eval.Evaluate(doc) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation, got: %v", err) + } + + // Get block IDs + blocks := doc.GetBlocks() + var bBlockID string + for _, node := range blocks { + if cb, ok := node.Block.(*document.CalcBlock); ok { + if strings.Contains(strings.Join(cb.Source(), ""), "b = 10") { + bBlockID = node.ID + } + } + } + + if bBlockID == "" { + t.Fatal("Could not find b block") + } + + // Re-evaluate via EvaluateBlock — should still succeed for b even though a failed + err = eval.EvaluateBlock(doc, bBlockID) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation from EvaluateBlock, got: %v", err) + } + + // Block b should have a result + bNode, _ := doc.GetBlock(bBlockID) + bBlock := bNode.Block.(*document.CalcBlock) + if bBlock.Error() != nil { + t.Errorf("Block b should not have error, got: %v", bBlock.Error()) + } + if bBlock.LastValue() == nil || bBlock.LastValue().String() != "10" { + t.Errorf("Expected b=10, got %v", bBlock.LastValue()) + } +} + +// TestBlockRecovery_EvaluateAffectedBlocksRecovery tests that EvaluateAffectedBlocks +// continues past a block error and evaluates the remaining blocks. +func TestBlockRecovery_EvaluateAffectedBlocksRecovery(t *testing.T) { + source := "a = 10\n\n\nb = 20\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if err != nil { + t.Fatalf("Initial Evaluate failed: %v", err) + } + + // Get block IDs + blocks := doc.GetBlocks() + var aBlockID, bBlockID string + for _, node := range blocks { + if cb, ok := node.Block.(*document.CalcBlock); ok { + src := strings.Join(cb.Source(), "") + if strings.Contains(src, "a = 10") { + aBlockID = node.ID + } + if strings.Contains(src, "b = 20") { + bBlockID = node.ID + } + } + } + + // Change block a to have an error + _, err = doc.ReplaceBlockSource(aBlockID, []string{"a = unknown_var"}) + if err != nil { + t.Fatalf("ReplaceBlockSource failed: %v", err) + } + + // EvaluateAffectedBlocks for both blocks + err = eval.EvaluateAffectedBlocks(doc, []string{aBlockID, bBlockID}) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation, got: %v", err) + } + + // Block a should have error + aNode, _ := doc.GetBlock(aBlockID) + aBlock := aNode.Block.(*document.CalcBlock) + if aBlock.Error() == nil { + t.Error("Block a should have error after introducing unknown_var") + } + + // Block b should still succeed + bNode, _ := doc.GetBlock(bBlockID) + bBlock := bNode.Block.(*document.CalcBlock) + if bBlock.Error() != nil { + t.Errorf("Block b should not have error, got: %v", bBlock.Error()) + } +} + +// TestBlockRecovery_TransformsSkipErroredBlocks tests that applyTransforms +// skips errored blocks but transforms successful ones. +func TestBlockRecovery_TransformsSkipErroredBlocks(t *testing.T) { + // scale factor 2 with unit_categories including Weight so grams get scaled + source := "---\nscale:\n factor: 2\n unit_categories: [Mass]\n---\na = unknown_var\n\n\nb = 10 grams\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation, got: %v", err) + } + + // Find the blocks + blocks := doc.GetBlocks() + var aBlock, bBlock *document.CalcBlock + for _, node := range blocks { + if cb, ok := node.Block.(*document.CalcBlock); ok { + src := strings.Join(cb.Source(), "") + if strings.Contains(src, "unknown_var") { + aBlock = cb + } + if strings.Contains(src, "b = 10 grams") { + bBlock = cb + } + } + } + + if bBlock == nil { + t.Fatal("Could not find b block") + } + + // Errored block should NOT have been transformed (applyTransforms skips errored blocks) + if aBlock != nil && aBlock.Error() == nil { + t.Error("Block a should have error") + } + + // b = 10 grams, scaled by 2 → 20 grams + results := bBlock.Results() + if len(results) == 0 { + t.Fatal("b block has no results") + } + val := results[0].String() + if !strings.Contains(val, "20") { + t.Errorf("Expected b to be scaled to 20 (grams), got %q", val) + } +} + +// TestBlockRecovery_InterpolationResolveSuccessLeavesErrored tests that +// interpolation resolves {{var}} for successful vars but leaves unresolved +// for errored vars. +func TestBlockRecovery_InterpolationResolveSuccessLeavesErrored(t *testing.T) { + // Block 1: a = unknown (error) + // Block 2: b = 42 (success) + // TextBlock: references both + source := "a = unknown_var\n\n\nb = 42\n\n\nResult a: {{a}}, Result b: {{b}}\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation, got: %v", err) + } + + // Find the TextBlock + for _, node := range doc.GetBlocks() { + tb, ok := node.Block.(*document.TextBlock) + if !ok { + continue + } + interp := tb.InterpolatedSource() + for _, line := range interp { + // b should be resolved to "42", a should remain as {{a}} + if strings.Contains(line, "Result b: 42") { + if strings.Contains(line, "{{a}}") { + return // Success + } + t.Errorf("Expected {{a}} to remain unresolved, got: %s", line) + return + } + } + } + t.Error("expected interpolated text block with b=42") +} + +// TestBlockRecovery_SemanticErrorMarksVarsErrored tests that when a block has a semantic +// error, the defined variables are marked as errored so downstream blocks get cascading errors. +func TestBlockRecovery_SemanticErrorMarksVarsErrored(t *testing.T) { + // Block 1: x = 1, x = 2 (redefinition semantic error within single block) + // Block 2: y = x + 1 (should get cascading error since x is errored) + // Note: the semantic checker catches x = 1, x = 2 as redefinition + source := "x = 1\nx = 2\n\n\ny = x + 1\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument failed: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("Expected ErrPartialEvaluation, got: %v", err) + } + + // Block 1 should have error (redefinition) + blocks := doc.GetBlocks() + calcBlocks := make([]*document.CalcBlock, 0) + for _, node := range blocks { + if cb, ok := node.Block.(*document.CalcBlock); ok { + calcBlocks = append(calcBlocks, cb) + } + } + if len(calcBlocks) < 2 { + t.Fatalf("Expected at least 2 calc blocks, got %d", len(calcBlocks)) + } + + if calcBlocks[0].Error() == nil { + t.Error("Block 1 should have redefinition error") + } + + // Block 2 should have cascading error (x is errored) + if calcBlocks[1].Error() == nil { + t.Error("Block 2 should have cascading error from errored x") + } + diags := calcBlocks[1].Diagnostics() + foundCascading := false + for _, d := range diags { + if d.Code == "cascading_error" { + foundCascading = true + } + } + if !foundCascading { + t.Errorf("Expected cascading_error diagnostic on block 2, got: %v", diags) + } +} + +// --- Golden file integration tests for error recovery --- + +// TestGolden_MultiErrorRecovery tests the multi_error_recovery.cm golden file. +// Block 1: a=1/0 (error), b=10 (success). Block 2: c=a*2 (cascading). Block 3: d=42, e=50 (success). +func TestGolden_MultiErrorRecovery(t *testing.T) { + // Use headings to create separate blocks + source := "a = 1 / 0\nb = 10\n\n# Block 2\n\nc = a * 2\n\n# Block 3\n\nd = 42\ne = d + 8" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + + // Should return ErrPartialEvaluation + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("expected ErrPartialEvaluation, got: %v", err) + } + + // Find CalcBlocks only + var calcBlocks []*document.CalcBlock + for _, node := range doc.GetBlocks() { + if cb, ok := node.Block.(*document.CalcBlock); ok { + calcBlocks = append(calcBlocks, cb) + } + } + if len(calcBlocks) != 3 { + t.Fatalf("expected 3 calc blocks, got %d", len(calcBlocks)) + } + + // Block 1: mixed success/error + if calcBlocks[0].Error() == nil { + t.Error("block 1 should have an error") + } + results1 := calcBlocks[0].Results() + if len(results1) != 2 { + t.Fatalf("block 1: expected 2 results, got %d", len(results1)) + } + if results1[0] != nil { + t.Error("block 1: result[0] should be nil (a = 1/0)") + } + if results1[1] == nil { + t.Error("block 1: result[1] should be non-nil (b = 10)") + } else if results1[1].String() != "10" { + t.Errorf("block 1: result[1] = %s, want 10", results1[1].String()) + } + + // Block 2: cascading error + if calcBlocks[1].Error() == nil { + t.Error("block 2 should have an error (cascading)") + } + diags2 := calcBlocks[1].Diagnostics() + hasCascading := false + for _, d := range diags2 { + if d.Code == "cascading_error" && d.Severity == "hint" { + hasCascading = true + } + } + if !hasCascading { + t.Error("block 2 should have a cascading_error diagnostic with hint severity") + } + + // Block 3: fully successful + if calcBlocks[2].Error() != nil { + t.Errorf("block 3 should have no error, got: %v", calcBlocks[2].Error()) + } + results3 := calcBlocks[2].Results() + if len(results3) != 2 { + t.Fatalf("block 3: expected 2 results, got %d", len(results3)) + } + if results3[0] == nil || results3[0].String() != "42" { + t.Errorf("block 3: result[0] = %v, want 42", results3[0]) + } + if results3[1] == nil || results3[1].String() != "50" { + t.Errorf("block 3: result[1] = %v, want 50", results3[1]) + } +} + +// TestGolden_CascadingErrors tests multi-level cascading across blocks. +// Block 1: x=1/0 (root), y=x+10 (cascade), z=y*2 (cascade). +// Block 2: ok=100 (success). Block 3: result=z+ok (cascade). +func TestGolden_CascadingErrors(t *testing.T) { + // Use headings to create separate blocks + source := "x = 1 / 0\ny = x + 10\nz = y * 2\n\n# ok block\n\nok = 100\n\n# result block\n\nresult = z + ok" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("expected ErrPartialEvaluation, got: %v", err) + } + + // Find CalcBlocks only + var calcBlocks []*document.CalcBlock + for _, node := range doc.GetBlocks() { + if cb, ok := node.Block.(*document.CalcBlock); ok { + calcBlocks = append(calcBlocks, cb) + } + } + if len(calcBlocks) != 3 { + t.Fatalf("expected 3 calc blocks, got %d", len(calcBlocks)) + } + + // Block 1: x errors, y and z cascade + diags := calcBlocks[0].Diagnostics() + rootCauseCount := 0 + cascadingCount := 0 + for _, d := range diags { + if d.Code == "eval_error" { + rootCauseCount++ + } + if d.Code == "cascading_error" && d.Severity == "hint" { + cascadingCount++ + } + } + if rootCauseCount != 1 { + t.Errorf("block 1: expected 1 root-cause error, got %d", rootCauseCount) + } + if cascadingCount != 2 { + t.Errorf("block 1: expected 2 cascading errors (y, z), got %d", cascadingCount) + } + + // Block 2: ok=100 succeeds + if calcBlocks[1].Error() != nil { + t.Errorf("block 2 (ok=100) should succeed, got: %v", calcBlocks[1].Error()) + } + if calcBlocks[1].LastValue() == nil || calcBlocks[1].LastValue().String() != "100" { + t.Errorf("block 2: LastValue = %v, want 100", calcBlocks[1].LastValue()) + } + + // Block 3: result=z+ok cascades + if calcBlocks[2].Error() == nil { + t.Error("block 3 (result=z+ok) should have cascading error") + } + hasCascading := false + for _, d := range calcBlocks[2].Diagnostics() { + if d.Code == "cascading_error" { + hasCascading = true + } + } + if !hasCascading { + t.Error("block 3 should have cascading_error diagnostic") + } +} + +// TestGolden_MixedSuccessAndErrors tests partial results with div-by-zero mid-document. +func TestGolden_MixedSuccessAndErrors(t *testing.T) { + source := "price = 25.00\ntax_rate = 0\ntax = price / tax_rate\nsubtotal = price + tax\nshipping = 5.99\ntotal = subtotal + shipping\ndiscount = 10\nfinal = total - discount" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + + eval := NewEvaluator() + err = eval.Evaluate(doc) + if !errors.Is(err, ErrPartialEvaluation) { + t.Fatalf("expected ErrPartialEvaluation, got: %v", err) + } + + blocks := doc.GetBlocks() + block := blocks[0].Block.(*document.CalcBlock) + results := block.Results() + + // 8 statements total + if len(results) != 8 { + t.Fatalf("expected 8 results, got %d", len(results)) + } + + // price=25.00 succeeds + if results[0] == nil { + t.Error("price should succeed") + } + // tax_rate=0 succeeds + if results[1] == nil { + t.Error("tax_rate should succeed") + } + // tax = price / tax_rate fails (div by zero) + if results[2] != nil { + t.Error("tax should be nil (division by zero)") + } + // subtotal cascades + if results[3] != nil { + t.Error("subtotal should be nil (cascading)") + } + // shipping=5.99 succeeds + if results[4] == nil { + t.Error("shipping should succeed") + } + // total cascades + if results[5] != nil { + t.Error("total should be nil (cascading)") + } + // discount=10 succeeds + if results[6] == nil { + t.Error("discount should succeed") + } + // final cascades + if results[7] != nil { + t.Error("final should be nil (cascading)") + } + + // Verify diagnostics + diags := block.Diagnostics() + rootCount := 0 + cascadeCount := 0 + for _, d := range diags { + if d.Code == "eval_error" { + rootCount++ + } + if d.Code == "cascading_error" { + cascadeCount++ + } + } + if rootCount != 1 { + t.Errorf("expected 1 root-cause error (tax), got %d", rootCount) + } + if cascadeCount != 3 { + t.Errorf("expected 3 cascading errors (subtotal, total, final), got %d", cascadeCount) + } +} + +// TestMultipleSemanticErrors verifies that ALL semantic errors in a block +// are reported, not just the first one. e.g., "a = 1; a = 2; c = 3; c = 5" +// should report redefinition errors for both 'a' (line 2) and 'c' (line 4). +func TestMultipleSemanticErrors(t *testing.T) { + source := "a = 1 / 0\na = 2\nc = 3\nc = 5\n" + + doc, err := document.NewDocument(source) + if err != nil { + t.Fatalf("NewDocument: %v", err) + } + + eval := NewEvaluator() + _ = eval.Evaluate(doc) + + blocks := doc.GetBlocks() + block := blocks[0].Block.(*document.CalcBlock) + + diags := block.Diagnostics() + + // Should have at least 2 redefinition diagnostics: one for 'a' and one for 'c' + redefCount := 0 + var redefVars []string + for _, d := range diags { + if d.Code == "variable_redefinition" { + redefCount++ + redefVars = append(redefVars, d.Message) + } + } + if redefCount < 2 { + t.Errorf("expected at least 2 redefinition diagnostics (for 'a' and 'c'), got %d: %v", redefCount, redefVars) + } +} diff --git a/impl/document/interpolation.go b/impl/document/interpolation.go index eace879a..34361ba0 100644 --- a/impl/document/interpolation.go +++ b/impl/document/interpolation.go @@ -89,7 +89,7 @@ func interpolateLine(line string, env map[string]types.Type, df display.Formatte if wrapHTML { return "\x02" + formatted + "\x03" } - return "**" + formatted + "**" + return formatted }) } diff --git a/impl/document/interpolation_test.go b/impl/document/interpolation_test.go index d3c34e42..a6f6f22c 100644 --- a/impl/document/interpolation_test.go +++ b/impl/document/interpolation_test.go @@ -18,7 +18,7 @@ func TestInterpolateLine(t *testing.T) { df := display.DefaultFormatter() got := interpolateLine("Result: {{x}}", env, df, nil, nil, false) - want := "Result: **42**" + want := "Result: 42" if got != want { t.Errorf("interpolateLine() = %q, want %q", got, want) } @@ -32,7 +32,7 @@ func TestInterpolateLineMultipleTags(t *testing.T) { df := display.DefaultFormatter() got := interpolateLine("{{a}} and {{b}}", env, df, nil, nil, false) - want := "**10** and **20**" + want := "10 and 20" if got != want { t.Errorf("interpolateLine() = %q, want %q", got, want) } @@ -74,9 +74,9 @@ func TestInterpolateLineDisplayFormatted(t *testing.T) { input string want string }{ - {"Total: {{cost}}", "Total: **$1.2M**"}, - {"Margin: {{pct}}", "Margin: **28%**"}, - {"Team: {{widgets}}", "Team: **14 people**"}, + {"Total: {{cost}}", "Total: $1.2M"}, + {"Margin: {{pct}}", "Margin: 28%"}, + {"Team: {{widgets}}", "Team: 14 people"}, } for _, tt := range tests { got := interpolateLine(tt.input, env, df, nil, nil, false) @@ -94,7 +94,7 @@ func TestInterpolateLineAdjacentTags(t *testing.T) { df := display.DefaultFormatter() got := interpolateLine("{{a}}{{b}}", env, df, nil, nil, false) - want := "**1****2**" + want := "12" if got != want { t.Errorf("interpolateLine() = %q, want %q", got, want) } @@ -107,7 +107,7 @@ func TestInterpolateLineInTable(t *testing.T) { df := display.DefaultFormatter() got := interpolateLine("| Revenue | {{rev}} |", env, df, nil, nil, false) - want := "| Revenue | **$4.2M** |" + want := "| Revenue | $4.2M |" if got != want { t.Errorf("interpolateLine() = %q, want %q", got, want) } @@ -120,7 +120,7 @@ func TestInterpolateLineInHeading(t *testing.T) { df := display.DefaultFormatter() got := interpolateLine("# Summary: {{total}}", env, df, nil, nil, false) - want := "# Summary: **500**" + want := "# Summary: 500" if got != want { t.Errorf("interpolateLine() = %q, want %q", got, want) } @@ -136,10 +136,10 @@ func TestInterpolateLineWhitespace(t *testing.T) { input string want string }{ - {"{{ x }}", "**99**"}, - {"{{ x }}", "**99**"}, - {"{{ x}}", "**99**"}, - {"{{x }}", "**99**"}, + {"{{ x }}", "99"}, + {"{{ x }}", "99"}, + {"{{ x}}", "99"}, + {"{{x }}", "99"}, } for _, tt := range tests { got := interpolateLine(tt.input, env, df, nil, nil, false) @@ -181,10 +181,10 @@ func TestInterpolateLineBackticks(t *testing.T) { input string want string }{ - {"`{{x}}`", "**42**"}, // Backticks stripped, bold wrapped - {"Value: `{{x}}`", "Value: **42**"}, // Inline backtick-wrapped - {"`{{ x }}`", "**42**"}, // Whitespace + backticks - {"{{x}}", "**42**"}, // No backticks still works + {"`{{x}}`", "42"}, // Backticks stripped, bold wrapped + {"Value: `{{x}}`", "Value: 42"}, // Inline backtick-wrapped + {"`{{ x }}`", "42"}, // Whitespace + backticks + {"{{x}}", "42"}, // No backticks still works {"`{{unknown}}`", "`{{unknown}}`"}, // Missing var: backticks preserved } for _, tt := range tests { @@ -211,7 +211,7 @@ func TestInterpolateLineWithTransform(t *testing.T) { got := interpolateLine("Total: {{cost}}", env, df, fm, nil, false) // 500 * 1000 = 500,000 → formatted as $500K - want := "Total: **$500K**" + want := "Total: $500K" if got != want { t.Errorf("interpolateLine() with scale = %q, want %q", got, want) } @@ -238,7 +238,7 @@ func TestInterpolateTextBlocks(t *testing.T) { continue } got := tb.InterpolatedSource() - if slices.Contains(got, "Total: **42**") { + if slices.Contains(got, "Total: 42") { // Raw source unchanged if slices.Contains(tb.Source(), "Total: {{x}}") { return // Success @@ -246,7 +246,7 @@ func TestInterpolateTextBlocks(t *testing.T) { t.Error("Source() should still contain {{x}}") return } - t.Errorf("InterpolatedSource() should contain 'Total: **42**', got %v", got) + t.Errorf("InterpolatedSource() should contain 'Total: 42', got %v", got) return } t.Error("expected a TextBlock in document") @@ -296,8 +296,8 @@ func TestInterpolateTextBlocksHTMLSource(t *testing.T) { } // Plain source should have bold wrapping plain := tb.InterpolatedSource() - if plain[0] != "Total: **42**" { - t.Errorf("InterpolatedSource()[0] = %q, want %q", plain[0], "Total: **42**") + if plain[0] != "Total: 42" { + t.Errorf("InterpolatedSource()[0] = %q, want %q", plain[0], "Total: 42") } // HTML source should have sentinels (no bold) html := tb.InterpolatedHTMLSourceText() @@ -347,7 +347,7 @@ func TestInterpolateLineDirectiveScale(t *testing.T) { } got := interpolateLine("Scale factor: {{ @scale }}", env, df, fm, nil, false) - want := "Scale factor: **3**" + want := "Scale factor: 3" if got != want { t.Errorf("interpolateLine(@scale) = %q, want %q", got, want) } @@ -364,7 +364,7 @@ func TestInterpolateLineDirectiveGlobals(t *testing.T) { parsed, _ := document.ParseGlobals(fm.Globals) got := interpolateLine("Tax: {{ @globals.tax_rate }}", env, df, fm, parsed.Values, false) - want := "Tax: **0.32**" + want := "Tax: 0.32" if got != want { t.Errorf("interpolateLine(@globals.tax_rate) = %q, want %q", got, want) } @@ -395,8 +395,28 @@ func TestInterpolateLineDirectiveNotScaled(t *testing.T) { got := interpolateLine("Factor: {{ @scale }}", env, df, fm, nil, false) // Should be 1000 (displayed as 1K), NOT 1000*1000=1000000 (1M) - want := "Factor: **1K**" + want := "Factor: 1K" if got != want { t.Errorf("interpolateLine(@scale, should not double-scale) = %q, want %q", got, want) } } + +// TestInterpolateLine_BoldSourceDoesNotDouble verifies that when the user +// writes **{{ var }}** in their markdown (already bold), the interpolation +// does not produce ****value**** (doubled bold markers). +func TestInterpolateLine_BoldSourceDoesNotDouble(t *testing.T) { + env := map[string]types.Type{ + "out": types.NewNumber(decimal.NewFromInt(42)), + } + df := display.DefaultFormatter() + + got := interpolateLine("We leave on **{{ out }}**", env, df, nil, nil, false) + // Should be 42, not **42** + if strings.Contains(got, "****") { + t.Errorf("interpolateLine() doubled bold markers: %q", got) + } + // The value should appear bold (either from user's ** or from interpolation, not both) + if !strings.Contains(got, "42") { + t.Errorf("interpolateLine() should contain 42, got: %q", got) + } +} diff --git a/impl/interpreter/environment.go b/impl/interpreter/environment.go index 70624a32..9bf2ad96 100644 --- a/impl/interpreter/environment.go +++ b/impl/interpreter/environment.go @@ -13,6 +13,7 @@ import ( type Environment struct { vars map[string]types.Type exchangeRates map[string]decimal.Decimal // "USD_EUR" -> rate + erroredVars map[string]error // variables that failed evaluation } // NewEnvironment creates a new empty environment with built-in constants. @@ -20,6 +21,7 @@ func NewEnvironment() *Environment { env := &Environment{ vars: make(map[string]types.Type), exchangeRates: make(map[string]decimal.Decimal), + erroredVars: make(map[string]error), } // Add built-in constants @@ -64,15 +66,49 @@ func (e *Environment) Clone() *Environment { newEnv := &Environment{ vars: make(map[string]types.Type), exchangeRates: make(map[string]decimal.Decimal), + erroredVars: make(map[string]error), } maps.Copy(newEnv.vars, e.vars) maps.Copy(newEnv.exchangeRates, e.exchangeRates) + maps.Copy(newEnv.erroredVars, e.erroredVars) return newEnv } -// GetAllVariables returns the map of all variables (for sync with semantic checker). +// GetAllVariables returns a snapshot copy of all variables. +// The returned map is safe to iterate without concern for concurrent mutation. func (e *Environment) GetAllVariables() map[string]types.Type { - return e.vars + result := make(map[string]types.Type, len(e.vars)) + maps.Copy(result, e.vars) + return result +} + +// SetError marks a variable as having failed evaluation. +func (e *Environment) SetError(name string, err error) { + e.erroredVars[name] = err +} + +// GetError checks if a variable has a recorded evaluation error. +// Returns the error and true if found, nil and false otherwise. +func (e *Environment) GetError(name string) (error, bool) { + err, ok := e.erroredVars[name] + return err, ok +} + +// ClearError removes a single variable's error state. +func (e *Environment) ClearError(name string) { + delete(e.erroredVars, name) +} + +// ClearErrors removes all tracked variable errors. +func (e *Environment) ClearErrors() { + clear(e.erroredVars) +} + +// GetAllErroredVars returns a copy of the errored variables map. +func (e *Environment) GetAllErroredVars() map[string]error { + result := make(map[string]error, len(e.erroredVars)) + maps.Copy(result, e.erroredVars) + return result } // SetExchangeRate sets an exchange rate for currency conversion. diff --git a/impl/interpreter/environment_test.go b/impl/interpreter/environment_test.go index 84c36639..4678b0d8 100644 --- a/impl/interpreter/environment_test.go +++ b/impl/interpreter/environment_test.go @@ -1,6 +1,7 @@ package interpreter import ( + "errors" "testing" "github.com/CalcMark/go-calcmark/spec/types" @@ -56,3 +57,126 @@ func TestEnvironmentSharing(t *testing.T) { t.Error("Value for 'a' is nil in second check") } } + +func TestEnvironment_SetError_GetError(t *testing.T) { + env := NewEnvironment() + evalErr := errors.New("division by zero") + + env.SetError("x", evalErr) + + got, ok := env.GetError("x") + if !ok { + t.Fatal("expected GetError to return true for errored variable") + } + if got != evalErr { + t.Fatalf("expected error %q, got %q", evalErr, got) + } +} + +func TestEnvironment_GetError_NonErrored(t *testing.T) { + env := NewEnvironment() + + got, ok := env.GetError("x") + if ok { + t.Fatal("expected GetError to return false for non-errored variable") + } + if got != nil { + t.Fatalf("expected nil error, got %q", got) + } +} + +func TestEnvironment_ClearError(t *testing.T) { + env := NewEnvironment() + env.SetError("x", errors.New("fail")) + + env.ClearError("x") + + got, ok := env.GetError("x") + if ok { + t.Fatal("expected GetError to return false after ClearError") + } + if got != nil { + t.Fatalf("expected nil error after ClearError, got %q", got) + } +} + +func TestEnvironment_ClearErrors(t *testing.T) { + env := NewEnvironment() + env.SetError("a", errors.New("err a")) + env.SetError("b", errors.New("err b")) + + env.ClearErrors() + + if _, ok := env.GetError("a"); ok { + t.Error("expected 'a' error to be cleared") + } + if _, ok := env.GetError("b"); ok { + t.Error("expected 'b' error to be cleared") + } +} + +func TestEnvironment_SetError_And_Set_Coexist(t *testing.T) { + env := NewEnvironment() + evalErr := errors.New("initial failure") + + env.SetError("x", evalErr) + env.Set("x", types.NewNumber(decimal.NewFromInt(42))) + + // Both should be present independently + val, valOk := env.Get("x") + if !valOk { + t.Fatal("expected Get to return true for variable with both value and error") + } + if val == nil { + t.Fatal("expected non-nil value") + } + + errVal, errOk := env.GetError("x") + if !errOk { + t.Fatal("expected GetError to still return true after Set") + } + if errVal != evalErr { + t.Fatalf("expected error %q, got %q", evalErr, errVal) + } +} + +func TestEnvironment_Clone_ErroredVars(t *testing.T) { + env := NewEnvironment() + origErr := errors.New("original error") + env.SetError("x", origErr) + + cloned := env.Clone() + + // Clone should have the error + got, ok := cloned.GetError("x") + if !ok { + t.Fatal("expected cloned env to have error for 'x'") + } + if got != origErr { + t.Fatalf("expected cloned error %q, got %q", origErr, got) + } + + // Mutating clone should not affect original + cloned.SetError("y", errors.New("clone-only")) + cloned.ClearError("x") + + if _, ok := env.GetError("y"); ok { + t.Error("original env should not have 'y' error added to clone") + } + if _, ok := env.GetError("x"); !ok { + t.Error("original env should still have 'x' error after clone cleared it") + } +} + +func TestEnvironment_GetError_UnsetVariable(t *testing.T) { + env := NewEnvironment() + + // Variable never set, never errored + got, ok := env.GetError("nonexistent") + if ok { + t.Fatal("expected GetError to return false for unset variable") + } + if got != nil { + t.Fatalf("expected nil error for unset variable, got %q", got) + } +} diff --git a/impl/interpreter/errors.go b/impl/interpreter/errors.go new file mode 100644 index 00000000..0b8b6af9 --- /dev/null +++ b/impl/interpreter/errors.go @@ -0,0 +1,18 @@ +package interpreter + +import "fmt" + +// CascadingError indicates that evaluation failed because a referenced +// variable has an error from a prior evaluation failure. +type CascadingError struct { + VarName string // the errored variable that was referenced + Cause error // the original error on that variable +} + +func (e *CascadingError) Error() string { + return fmt.Sprintf("depends on errored variable %q: %s", e.VarName, e.Cause) +} + +func (e *CascadingError) Unwrap() error { + return e.Cause +} diff --git a/impl/interpreter/interpreter_test.go b/impl/interpreter/interpreter_test.go index 05453577..c6b94280 100644 --- a/impl/interpreter/interpreter_test.go +++ b/impl/interpreter/interpreter_test.go @@ -1,6 +1,8 @@ package interpreter import ( + "errors" + "fmt" "testing" "github.com/CalcMark/go-calcmark/spec/ast" @@ -278,3 +280,137 @@ func TestBuiltinConstantsUsage(t *testing.T) { t.Errorf("2 * PI = %v, want ~6.28", val) } } + +// Test CascadingError type + +func TestCascadingError_ErrorMessage(t *testing.T) { + cause := fmt.Errorf("division by zero") + ce := &CascadingError{VarName: "x", Cause: cause} + + want := `depends on errored variable "x": division by zero` + if got := ce.Error(); got != want { + t.Errorf("CascadingError.Error() = %q, want %q", got, want) + } +} + +func TestCascadingError_Unwrap(t *testing.T) { + cause := fmt.Errorf("division by zero") + ce := &CascadingError{VarName: "x", Cause: cause} + + if unwrapped := ce.Unwrap(); unwrapped != cause { + t.Errorf("CascadingError.Unwrap() = %v, want %v", unwrapped, cause) + } +} + +func TestCascadingError_ErrorsAs(t *testing.T) { + cause := fmt.Errorf("division by zero") + var err error = &CascadingError{VarName: "x", Cause: cause} + + var ce *CascadingError + if !errors.As(err, &ce) { + t.Fatal("errors.As failed to match CascadingError") + } + if ce.VarName != "x" { + t.Errorf("VarName = %q, want %q", ce.VarName, "x") + } +} + +// Test evalIdentifier with errored variables + +func TestEvalIdentifier_ErroredVariable(t *testing.T) { + interp := NewInterpreter() + cause := fmt.Errorf("division by zero") + interp.env.SetError("x", cause) + + id := &ast.Identifier{Name: "x"} + _, err := interp.evalIdentifier(id) + if err == nil { + t.Fatal("expected error for errored variable, got nil") + } + + var ce *CascadingError + if !errors.As(err, &ce) { + t.Fatalf("expected CascadingError, got %T: %v", err, err) + } + if ce.VarName != "x" { + t.Errorf("VarName = %q, want %q", ce.VarName, "x") + } + if ce.Cause != cause { + t.Errorf("Cause = %v, want %v", ce.Cause, cause) + } +} + +func TestEvalIdentifier_NormalVariable_Unchanged(t *testing.T) { + interp := NewInterpreter() + interp.env.Set("y", types.NewNumber(decimal.NewFromInt(42))) + + id := &ast.Identifier{Name: "y"} + result, err := interp.evalIdentifier(id) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result.String() != "42" { + t.Errorf("result = %v, want 42", result.String()) + } +} + +func TestEvalIdentifier_ErroredTakesPrecedenceOverValue(t *testing.T) { + interp := NewInterpreter() + cause := fmt.Errorf("some error") + // Variable has both a value and an error; error should take precedence + interp.env.Set("x", types.NewNumber(decimal.NewFromInt(10))) + interp.env.SetError("x", cause) + + id := &ast.Identifier{Name: "x"} + _, err := interp.evalIdentifier(id) + if err == nil { + t.Fatal("expected CascadingError when variable is errored, even if value exists") + } + + var ce *CascadingError + if !errors.As(err, &ce) { + t.Fatalf("expected CascadingError, got %T: %v", err, err) + } +} + +func TestEvalIdentifier_CascadingErrorInBinaryOp(t *testing.T) { + interp := NewInterpreter() + cause := fmt.Errorf("division by zero") + interp.env.SetError("a", cause) + interp.env.Set("b", types.NewNumber(decimal.NewFromInt(5))) + + // Expression: a + b (where a is errored) + node := &ast.BinaryOp{ + Operator: "+", + Left: &ast.Identifier{Name: "a"}, + Right: &ast.Identifier{Name: "b"}, + } + + _, err := interp.evalBinaryOp(node) + if err == nil { + t.Fatal("expected error from binary op with errored variable") + } + + var ce *CascadingError + if !errors.As(err, &ce) { + t.Fatalf("expected CascadingError, got %T: %v", err, err) + } + if ce.VarName != "a" { + t.Errorf("VarName = %q, want %q", ce.VarName, "a") + } +} + +func TestEvalIdentifier_UndefinedVariable_NotCascading(t *testing.T) { + interp := NewInterpreter() + + id := &ast.Identifier{Name: "nonexistent"} + _, err := interp.evalIdentifier(id) + if err == nil { + t.Fatal("expected error for undefined variable") + } + + var ce *CascadingError + if errors.As(err, &ce) { + t.Error("undefined variable error should NOT be a CascadingError") + } +} diff --git a/impl/interpreter/variables.go b/impl/interpreter/variables.go index 8a800a12..d4ae9924 100644 --- a/impl/interpreter/variables.go +++ b/impl/interpreter/variables.go @@ -16,11 +16,17 @@ func (interp *Interpreter) evalAssignment(a *ast.Assignment) (types.Type, error) } interp.env.Set(a.Name, value) + interp.env.ClearError(a.Name) // Clear any stale error from a prior failed evaluation return value, nil } func (interp *Interpreter) evalIdentifier(id *ast.Identifier) (types.Type, error) { - // Check for defined variables FIRST (variables take precedence over keywords) + // Check for errored variables FIRST (error recovery: variable failed in prior statement) + if cause, errored := interp.env.GetError(id.Name); errored { + return nil, &CascadingError{VarName: id.Name, Cause: cause} + } + + // Check for defined variables (variables take precedence over keywords) if value, ok := interp.env.Get(id.Name); ok { return value, nil } diff --git a/site/assets/css/components.css b/site/assets/css/components.css index 0f81ac92..ff3ed4c6 100644 --- a/site/assets/css/components.css +++ b/site/assets/css/components.css @@ -782,6 +782,16 @@ h6:hover .heading-anchor { color: var(--color-warning); } +.cm-results.cm-partial-error { + border-color: color-mix(in srgb, var(--color-warning) 50%, var(--color-border)); +} + +.cm-result-error .cm-error-value { + color: var(--color-warning); + font-style: italic; + font-weight: 400; +} + /* Repo file links with copy button */ .repo-file { display: inline-flex; diff --git a/site/layouts/_default/_markup/render-codeblock-calcmark.html b/site/layouts/_default/_markup/render-codeblock-calcmark.html index c4109216..5c59633f 100644 --- a/site/layouts/_default/_markup/render-codeblock-calcmark.html +++ b/site/layouts/_default/_markup/render-codeblock-calcmark.html @@ -21,33 +21,39 @@ {{ transform.Highlight $raw "text" (dict "lineNos" false) }} {{- with $results -}} - {{- if .error -}} + {{- /* Check if there are any displayable results or errors */ -}} + {{- $hasResults := false -}} + {{- range .lines -}} + {{- if or .result .error -}}{{ $hasResults = true }}{{- end -}} + {{- end -}} + + {{- if $hasResults -}} +
+
Results
+ + {{- range .lines -}} + {{- if .result -}} + + + + + + {{- else if .error -}} + + + + + + {{- end -}} + {{- end -}} +
{{ .source }}{{ .result }}
{{ .source }}{{ .error }}
+
+ {{- else if .error -}} + {{- /* Fatal error with no results at all */ -}}
Error
{{ .error }}
- {{- else -}} - {{- /* Only show results div if there are actual results */ -}} - {{- $hasResults := false -}} - {{- range .lines -}} - {{- if .result -}}{{ $hasResults = true }}{{- end -}} - {{- end -}} - {{- if $hasResults -}} -
-
Results
- - {{- range .lines -}} - {{- if .result -}} - - - - - - {{- end -}} - {{- end -}} -
{{ .source }}{{ .result }}
-
- {{- end -}} {{- end -}} {{- end -}}
diff --git a/testdata/embedded/complex_markdown.expected.md b/testdata/embedded/complex_markdown.expected.md index 4fc17698..68e5781f 100644 --- a/testdata/embedded/complex_markdown.expected.md +++ b/testdata/embedded/complex_markdown.expected.md @@ -79,7 +79,11 @@ monthly_bandwidth = bandwidth_gbps * cost_per_gbps → $600.00 This block references an undefined variable. The preprocessor should emit an inline error and continue processing the rest of the document. -> **CalcMark Error:** line 1: undefined_variable: undefined variable "nonexistent_var" (line 83) +```calcmark +broken_total = nonexistent_var + 100 +``` + +**Error:** line 1: undefined_variable: undefined variable "nonexistent_var" ## Markdown Feature Showcase diff --git a/testdata/eval/errors/features/cascading_errors.cm b/testdata/eval/errors/features/cascading_errors.cm new file mode 100644 index 00000000..d15a5806 --- /dev/null +++ b/testdata/eval/errors/features/cascading_errors.cm @@ -0,0 +1,17 @@ +# Cascading Errors + +When a variable fails, all downstream variables that reference it get cascading errors with hint severity, distinguishing them from root-cause errors. + +## Root cause and cascade within one block + +x = 1 / 0 +y = x + 10 +z = y * 2 + +## Independent success + +ok = 100 + +## Multi-level cascade across blocks + +result = z + ok diff --git a/testdata/eval/errors/features/mixed_success_and_errors.cm b/testdata/eval/errors/features/mixed_success_and_errors.cm new file mode 100644 index 00000000..0814eae5 --- /dev/null +++ b/testdata/eval/errors/features/mixed_success_and_errors.cm @@ -0,0 +1,14 @@ +# Mixed Success and Errors + +A document where some lines succeed and some fail, demonstrating that successful computations show values even when other lines have errors. + +## Calculations + +price = 25.00 +tax_rate = 0 +tax = price / tax_rate +subtotal = price + tax +shipping = 5.99 +total = subtotal + shipping +discount = 10 +final = total - discount diff --git a/testdata/eval/errors/features/multi_error_recovery.cm b/testdata/eval/errors/features/multi_error_recovery.cm new file mode 100644 index 00000000..c74ba597 --- /dev/null +++ b/testdata/eval/errors/features/multi_error_recovery.cm @@ -0,0 +1,17 @@ +# Multi-Error Recovery + +Error recovery allows the evaluator to continue past errors and show all diagnostics simultaneously. + +## Block 1: Division by zero + +a = 1 / 0 +b = 10 + +## Block 2: References errored variable from block 1 + +c = a * 2 + +## Block 3: Independent (no errors) + +d = 42 +e = d + 8