internal/controller/ledger/controller_default.go (1)

406-433: Schema enforcement: avoid double-reporting + avoid high-cardinality trace attributes

In audit mode, Template == "" currently triggers an error report, then immediately falls into the “template not found” path (another error report), and then proceeds to parse/execute whatever Plain is (possibly empty).

Also attribute.String("schema_validation_failed", err.Error()) risks high-cardinality attributes (template IDs / full error strings).

Suggested shape:

 	if schema != nil {
-		if parameters.Input.Template == "" {
-			err := newErrSchemaValidationError(parameters.SchemaVersion, fmt.Errorf("transactions on this ledger must use a template"))
-			if ctrl.schemaEnforcementMode == SchemaEnforcementStrict {
-				return nil, err
-			} else {
-				trace.SpanFromContext(ctx).SetAttributes(attribute.String("schema_validation_failed", err.Error()))
-				logging.FromContext(ctx).Errorf("schema validation failed: %s", err)
-			}
-		}
-		if template, ok := schema.SchemaData.Transactions[parameters.Input.Template]; ok {
+		if parameters.Input.Template == "" {
+			err := newErrSchemaValidationError(parameters.SchemaVersion, fmt.Errorf("transactions on this ledger must use a template"))
+			if ctrl.schemaEnforcementMode == SchemaEnforcementStrict {
+				return nil, err
+			}
+			trace.SpanFromContext(ctx).SetAttributes(
+				attribute.Bool("schema_validation_failed", true),
+				attribute.String("schema_validation_reason", "missing_template"),
+			)
+			logging.FromContext(ctx).Errorf("schema validation failed: %s", err)
+		} else if template, ok := schema.SchemaData.Transactions[parameters.Input.Template]; ok {
 			parameters.Input.Plain = template.Script
 			if parameters.Input.Runtime == "" {
 				parameters.Input.Runtime = template.Runtime
 			}
 		} else {
 			err := newErrSchemaValidationError(parameters.SchemaVersion, fmt.Errorf("failed to find transaction template `%s`", parameters.Input.Template))
 			if ctrl.schemaEnforcementMode == SchemaEnforcementStrict {
 				return nil, err
 			} else {
-				trace.SpanFromContext(ctx).SetAttributes(attribute.String("schema_validation_failed", err.Error()))
+				trace.SpanFromContext(ctx).SetAttributes(
+					attribute.Bool("schema_validation_failed", true),
+					attribute.String("schema_validation_reason", "unknown_template"),
+				)
 				logging.FromContext(ctx).Errorf("schema validation failed: %s", err)
 			}
 		}
 	}

Also: please confirm the intended spec in audit mode—should ad-hoc scripts still run when a schema is present, or should it be “template-only” regardless? (This is the same concern raised previously.) Based on past review comments, …

internal/transaction.go (1)

37-53: Consider making Transaction.Template nullable (nullzero or *string) to avoid persisting empty-string “unset”.
Not required, but it reduces ambiguity between “no template (legacy)” vs “template id accidentally empty”.

internal/storage/ledger/resource_transactions.go (1)

35-52: Including template in the projection is the right move; consider adding it to the resource schema if filtering is desired.

internal/api/v2/controllers_transactions_create.go (1)

20-35: Multi-type validation is good; improve the error to report all conflicting types (not just first two).
Currently if all three are provided, the message only mentions the first two.

 import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 ...
 )
 ...
 		if len(txType) > 1 {
-			api.BadRequest(w, common.ErrValidation, fmt.Errorf("cannot pass %v and %v in the same request", txType[0], txType[1]))
+			api.BadRequest(w, common.ErrValidation, fmt.Errorf("cannot mix transaction types in the same request: %s", strings.Join(txType, ", ")))
 			return
 		} else if len(txType) == 0 {

test/e2e/api_transactions_create_test.go (1)

102-111: Pointer migration looks consistent; consider adding at least one schema+template create test.
The pointer.For(...) updates align with Plain becoming nullable. Given the feature, it’d be valuable to add an e2e case that (1) inserts a schema with templates and (2) creates a transaction via script.template (and asserts template info in the resulting transaction/event).

Also applies to: 316-330, 489-501, 519-536, 552-562, 577-593, 602-618, 627-640

pkg/client/models/components/runtime.go (1)

10-35: Nice strict enum parsing; consider adding IsValid() (and optionally MarshalJSON) for reuse.
This can reduce duplicated switch logic elsewhere and make validation callable without JSON roundtrips.

internal/transaction_templates.go (1)

23-29: Avoid per-iteration allocations in TransactionTemplates.Validate()

[]RuntimeType{"", ...} is allocated on each loop iteration; also the error message doesn’t mention that "" is accepted.

 func (t TransactionTemplates) Validate() error {
 	for _, t := range t {
-		if !slices.Contains([]RuntimeType{"", RuntimeMachine, RuntimeExperimentalInterpreter}, t.Runtime) {
-			return fmt.Errorf("unexpected runtime `%s`: should be `%s` or `%s`", t.Runtime, RuntimeMachine, RuntimeExperimentalInterpreter)
-		}
+		switch t.Runtime {
+		case "", RuntimeMachine, RuntimeExperimentalInterpreter:
+			// ok
+		default:
+			return fmt.Errorf(
+				"unexpected runtime %q: should be %q or %q (or omitted)",
+				t.Runtime, RuntimeMachine, RuntimeExperimentalInterpreter,
+			)
+		}
 	}
 	return nil
 }

internal/controller/ledger/controller_default_test.go (2)

26-83: TestCreateTransactionWithoutSchema: consider using a non-empty RunScript.Plain

Right now Parse(runScript.Plain) is parsing "". Even though it’s mocked, using a minimal valid script makes the test intent clearer and reduces “false confidence” if this ever stops being a pure mock.

---

85-220: TestCreateTransactionWithSchema: make template script + mocked postings consistent

The template script sends EUR/2 while the mocked execution returns a USD posting; since the parser/runtime are mocked, this isn’t functionally wrong, but it’s confusing for future readers.

-	send [EUR/2 100] (
+	send [USD 100] (
 		source = world
 		destination = $dest
 	)

internal/controller/ledger/controller_default.go (4)

64-77: Schema template validation: prefer %q for template IDs (and drop redundant cast)

Minor polish: IDs are user-controlled strings; use %q. Also template.Runtime is already a ledger.RuntimeType.

 	for id, template := range schema.Transactions {
-		parser := ctrl.getParser(ledger.RuntimeType(template.Runtime))
+		parser := ctrl.getParser(template.Runtime)
 		_, err := parser.Parse(template.Script)
 		if err != nil {
-			return nil, ledger.NewErrInvalidSchema(fmt.Errorf("invalid template %s: %w", id, err))
+			return nil, ledger.NewErrInvalidSchema(fmt.Errorf("invalid template %q: %w", id, err))
 		}
 	}

---

437-446: Small cleanup: inline return m.Execute(...)

No behavior change—just removes a temporary.

---

481-487: Don’t persist an empty template string

WithTemplate(parameters.Input.Template) is called unconditionally. If empty means “not set”, consider guarding it to avoid storing "" (especially for no-schema flows).

 	transaction := ledger.NewTransaction().
 		WithPostings(result.Postings...).
 		WithMetadata(finalMetadata).
 		WithTimestamp(parameters.Input.Timestamp).
 		WithReference(parameters.Input.Reference).
-		WithTemplate(parameters.Input.Template)
+		WithTemplate(parameters.Input.Template)
+
+	// If `WithTemplate("")` is meaningful today, ignore this; otherwise:
+	// if parameters.Input.Template != "" { transaction = transaction.WithTemplate(parameters.Input.Template) }

---

573-583: Unused schema parameters: consider _schema to satisfy linters/conventions

Go allows unused parameters, but many linters/code-style conventions prefer _-prefixed names when intentionally unused. (You already do this in revertTransaction.)

Also applies to: 611-626, 633-644

test/e2e/api_schema_test.go (1)

446-470: Pointer construction inconsistency in ad-hoc script test

Elsewhere you use &components.V2PostTransactionScript{...}; here you use pointer.For(components.V2PostTransactionScript{...}). Both work, but mixing styles makes the test harder to scan.

-							Script: pointer.For(components.V2PostTransactionScript{
+							Script: &components.V2PostTransactionScript{
 								Plain: pointer.For(`
 								send [EUR/2] (
 									source = $users:001
 									destination = $bank:001
 								)`),
-							}),
+							},

Also applies to: 472-489

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

internal/machine/vm/run.go (1)

21-25: LGTM! Past review feedback addressed.

The omitempty tags have been correctly added to Plain, Template, and Vars fields as requested in the previous review, ensuring that empty fields are not emitted in JSON and preventing confusion between plain and template script modes.

docs/api/README.md (2)

3985-3985: Clarify template field semantics (unresolved from previous review).

The template field is added to V2Transaction and V2PostTransaction.script but lacks documentation on its presence/absence behavior. Prior feedback (lines 870–872) requested clarification on whether:

• (a) template is omitted/null when transaction is not from a template, or
• (b) template is always present (possibly empty string when not from a template)

Update field descriptions (e.g., at line 4007 and 4059) to explicitly state this behavior, and adjust all examples to consistently reflect the chosen semantics (e.g., remove template from non-template transaction examples or set it to null).

Also applies to: 4007-4007, 4059-4059

---

5784-5865: Address markdownlint MD024 duplicate heading errors (unresolved from previous review).

Static analysis flags duplicate headings at lines 5798, 5827, and 5860 (Runtime, V2TransactionTemplate, V2TransactionTemplates sections). Since this file is auto-generated by Widdershins, you have two options:

1. Regenerate the spec if duplicate heading definitions exist in the upstream OpenAPI specification.
2. Disable MD024 locally by wrapping the generated schema block with:

   <!-- markdownlint-disable MD024 -->
   [generated schema sections]
   <!-- markdownlint-enable MD024 -->

If regeneration is not feasible, add disable/enable comments around the schema definitions (before line 5784 and after line 5865) to suppress the linter.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.