Held-out correctness fixes: nested shorthand, short-circuit, map_get nil, list values

apps/release_authority/release_authority_test.go

@@ -85,12 +85,18 @@ func TestReleaseAuthority(t *testing.T) {
 				t.Fatalf("extra output after decision: %v", err)
 			}
 			var proposal struct {
-				Target string `json:"target"`
+				Target *string `json:"target"`
 			}
 			if err := json.Unmarshal([]byte(proposalJSON), &proposal); err != nil {
 				t.Fatal(err)
 			}
-			if decision["target"] != proposal.Target {
+			// A missing target is nil from map_get, so the app's is_nil
+			// fallback reports "unknown" (2026-10-12 nil semantics).
+			wantTarget := "unknown"
+			if proposal.Target != nil {
+				wantTarget = *proposal.Target
+			}
+			if decision["target"] != wantTarget {
 				t.Fatalf("target did not round-trip: %q", decision["target"])
 			}
 			tokens := json.NewDecoder(strings.NewReader(outStr))

apps/task_api/task_api.howl

@@ -26,7 +26,7 @@
         (res_json 400 (dict ("error" "invalid_json")))
       )
       (let (title (map_get body "title"))
-        (if (= title "")
+        (if (or (is_nil title) (= title ""))
           (res_json 400 (dict ("error" "title_required")))
           (let (id (call next_task_id))
             (let (task (dict ("id" id) ("title" title) ("status" "open")))
@@ -84,7 +84,7 @@
         (res_json 400 (dict ("error" "invalid_json")))
       )
       (let (id (map_get body "id"))
-        (if (= id "")
+        (if (or (is_nil id) (= id ""))
           (res_json 400 (dict ("error" "id_required")))
           (do
             (store_open kv "memory://session")
@@ -106,7 +106,7 @@
         (res_json 400 (dict ("error" "invalid_json")))
       )
       (let (id (map_get body "id"))
-        (if (= id "")
+        (if (or (is_nil id) (= id ""))
           (res_json 400 (dict ("error" "id_required")))
           (do
             (store_open kv "memory://session")
@@ -135,7 +135,7 @@
         (res_json 400 (dict ("error" "invalid_json")))
       )
       (let (id (map_get body "id"))
-        (if (= id "")
+        (if (or (is_nil id) (= id ""))
           (res_json 400 (dict ("error" "id_required")))
           (do
             (store_open kv "memory://session")

docs/journals/2026-10-12_heldout_correctness_fixes.md

@@ -0,0 +1,125 @@
+# 2026-10-12: Held-out correctness fixes
+
+Base: main `c21a029c` (#136 merged). This slice fixes the four correctness
+bugs the held-out benchmark (heldout-v0) surfaced. No new built-ins. No token
+claims. No production `-compile-bc`/HFIR flip.
+
+## 1. Nested shorthand under the governed profile
+
+**Bug:** shorthand (`str_replace`, `read_text`, `str`, ...) nested inside the
+argument of a value-taking built-in (`write_file`, `to_lower`, ...) was not
+expanded. The governed build then failed with "no lowering for construct".
+
+**Fix:** `internal/ast/desugar.go` `isSugarExprContext` now lists every
+value-taking built-in as a parent. These include write_file, read_file,
+to_lower/to_upper, str_trim, the escapes, regex_match, is_nil, map_keys, env,
+exec, store_*, the req_*/res_* readers, llm_generate and others. A new drift
+test (`desugar_context_drift_test.go`) walks the construct registry. Every
+supported, non-opaque construct must expand nested sugar unless it is listed as
+structural, with a written reason.
+
+**Fail-closed is unchanged:** the order is still pre-desugar profile scan,
+then desugar, then post-desugar `hfir.VerifyProfile`, then build. A forbidden
+construct inside a nested shorthand argument is rejected before any artifact
+exists, and nothing runs. `TestHeldoutNestedShorthandGovernedFailsClosed`
+covers this: an `exec` hidden in `str_replace` inside `write_file` yields no
+artifact and no side file.
+
+## 2. Short-circuit `and` / `or`
+
+**Bug:** both operands were always evaluated.
+
+**Fix:**
+- **AST interpreter:** `evalBinop` decides before it evaluates the right side.
+- **AST bytecode:** `bytecode.ShortCircuit` emits a JUMP_IF_FALSE guard around
+  the right operand. The HFIR bytecode lowering uses the same helper.
+- **Go/JS backends:** they already used native `&&`/`||`.
+
+The right operand of `and` runs only when the left is true, and the right of
+`or` only when the left is false. Effects (`write_file`) and errors
+(`to_int "qq"`) on the skipped side do not happen.
+
+## 3. `map_get` on a missing key returns nil (behavior change)
+
+**Before:** a miss returned `""`, so `is_nil` and `nz` defaults never fired on a
+miss.
+
+**Now:** a miss returns nil on every path:
+- **VM paths:** interpreter, bytecode and HFIR bytecode.
+- **JS:** returns `null`.
+- **Go:** returns nil for dynamic maps.
+
+Text rendering is kept stable. `vm.TextOf` renders nil as `""` in `print`,
+`stderr`, `to_string` and `str_join`. The Go/JS backends render nil/null as `""`
+in `print` and `to_string`. So `(print (map_get m "zz"))` still prints an empty
+field. `is_nil`, `nz` and `=` now tell a miss (`nil`) apart from a present `""`:
+`(= (map_get m "zz") "")` is now **false**.
+
+Other consequences:
+- A chained `map_get` through a missing intermediate is still `TYPE_ERROR`,
+  now "expected dict, got nil".
+- A missing `store_get` record now prints as an empty field instead of `<nil>`.
+  `is_nil` is the way to observe absence.
+
+**Audit and migration:**
+- `apps/task_api`: the title/id validation compared with `""`. It now uses
+  `(or (is_nil x) (= x ""))`, so it still rejects both a missing and an empty
+  value. Short-circuit makes this safe.
+- `apps/release_authority`: it already had `is_nil` fallbacks to `"unknown"`,
+  which were dead code before. They now fire. The test expects `"unknown"` when
+  a proposal omits `target`.
+- The vm/gogen/js absence and path tests were renamed `...MissIsNil` and their
+  expectations updated.
+- The lowered-HFIR ABI conformance fixture `map_get_absence` now expects
+  `is_nil` of a miss to be `true`.
+- A grep of all `.howl` for `(= (map_get …) "")` found nothing else.
+
+**Go typed-map caveat:** a Go `map[string]string` cannot hold nil, so a value
+read of a miss is still `""`. `is_nil (map_get m k)` on a named map is compiled
+as a key-presence check (`howlFrameMapGetIsNil`), so it agrees with the VM.
+`nz (map_get m k) d` on a typed string map in the Go target may still see `""`.
+
+## 4. Lists are values (decision)
+
+**Decision:** explicit value semantics, applied the same way on every target.
+- `append` rebinds only the named variable to a new list.
+- A list stored in a map, or bound to another name, is unchanged.
+- To update the map, `map_set` the list back.
+- Dicts keep their in-place `map_set` mutation, which is unchanged.
+
+The VM paths already copied on append. The bug was that the backends did not:
+- **JS:** used `push`, which aliased. `howlFrameAppend` now returns a copy and
+  the emitted code is `xs = howlFrameAppend(xs, item)`.
+- **Go:** `append` could write into a shared backing array. It now uses a full
+  slice expression, `append(xs[:len(xs):len(xs)], item)`, in both the direct
+  form and the helper.
+
+## Regression tests
+
+- `heldout_correctness_cli_test.go`: each bug runs on the AST interpreter
+  (`-run`), governed AST bytecode (`build --profile governed` then `-run-bc`)
+  and HFIR bytecode (`-compile-hfir-bc` then `-run-bc`). It includes the
+  effect-skipping short-circuit test with `write_file` on the skipped side.
+- `internal/backend/gogen/heldout_semantics_test.go` and
+  `internal/backend/javascript/heldout_semantics_test.go` cover list aliasing,
+  nil rendering, `is_nil` of a miss and short-circuit. Go dict literals are
+  string-valued, so the list-in-map case is covered on the VM and JS paths.
+- `desugar_context_drift_test.go` keeps the shorthand parent list complete.
+
+## Benchmarks (verification only; no claims)
+
+- **token-efficiency-v0 `run.sh`:** howl 60/60, python 60/60, js 60/60, go 60/60.
+- **token-efficiency-heldout-v0 `run.sh`:** exit 0. All 126 references verified
+  (howl 30, plus 32 each for python, js and go), with the same 2 recorded gaps
+  (h31, h32). The committed results files are unchanged.
+- **h05/h07/h12 natural forms:** rewritten in a scratch run with nested
+  `str_replace (read_text …)`, `or` with `to_int` on the right, and
+  `nz (map_get …) 0`. All three pass the verifier. They were **not** committed,
+  so the frozen solution hashes and the #136 report stay as they are.
+
+## Known gaps
+
+- The JS backend's `nz` lowering emits `let` in expression position, which is
+  a syntax error. This bug predates this slice and is untouched here.
+- Go dict literals cannot hold lists or ints (string-valued only). This also
+  predates this slice.

docs/reference/lowered_hfir_abi_v1.md

@@ -53,7 +53,7 @@ These grant nothing. The suite runs them with an empty capability grant.
 * `let` and `print`. `print` writes one line. Several arguments are separated by a single space.
 * `dict`, `map_get`, `map_keys`, `list`, `list_len`, `str_join`, `is_nil`, `html_escape`, `attr_escape`, `regex_match`.
 
-`(map_get dict key)` on a missing key is `""`. `is_nil` of that result is false. A present value is returned as stored. This is the #103 absence rule. `map_get` grants nothing.
+`(map_get dict key)` on a missing key is nil. `is_nil` of that result is true. Nil renders as `""` in `print` and `to_string`, so a miss still prints as an empty field. A present value is returned as stored. Go caveat: on a typed `map[string]string` a miss still reads as `""` (that map cannot hold nil); `is_nil` of a named `map_get` is compiled as a key-presence check so it still agrees with the VM. This supersedes the #103 empty-string miss rule. `map_get` grants nothing.
 
 `(map_keys dict)` returns a list of strings in UTF-8 byte order (Go `sort.Strings`, Go string `<`). An empty dict is an empty list, so its length is `0`. A value that is not a dict fails at runtime with `TYPE_ERROR` and the text `map_keys expected dict`. `map_keys` grants nothing. The order, including keys outside the Basic Multilingual Plane, is the #109 rule. The suite replays `tests/fixtures/map_keys_sort_bmp.howl` and `tests/fixtures/map_keys_sort_nonbmp.howl`. Wasm does not emit `map_keys`; #73 must use this byte order if it ever does.
 

heldout_correctness_cli_test.go

@@ -0,0 +1,163 @@
+package main
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Regression tests for the correctness bugs found by the held-out benchmark
+// (docs/journals/2026-10-12_heldout_correctness_fixes.md). Each program runs
+// on every applicable execution path: the AST interpreter, AST bytecode built
+// under the governed profile (-run-bc), and the experimental HFIR bytecode.
+
+type heldoutPath struct {
+	name string
+	run  func(t *testing.T, bin, dir, src string) (string, error)
+}
+
+func heldoutPaths() []heldoutPath {
+	caps := func(dir string) string {
+		return "filesystem:read=" + dir + ",filesystem:write=" + dir
+	}
+	runBC := func(bin, dir, art string) (string, error) {
+		out, err := exec.Command(bin, "-run-bc", "-allow-caps", caps(dir), art).CombinedOutput()
+		return string(out), err
+	}
+	return []heldoutPath{
+		{"interpreter", func(t *testing.T, bin, dir, src string) (string, error) {
+			out, err := exec.Command(bin, "-run", "-allow-caps", caps(dir), src).CombinedOutput()
+			return string(out), err
+		}},
+		{"bytecode_governed", func(t *testing.T, bin, dir, src string) (string, error) {
+			art := filepath.Join(dir, "g.hfbc")
+			if out, err := exec.Command(bin, "build", "--profile", "governed", src, "-o", art).CombinedOutput(); err != nil {
+				t.Fatalf("governed build: %v %s", err, out)
+			}
+			return runBC(bin, dir, art)
+		}},
+		{"hfir_bytecode", func(t *testing.T, bin, dir, src string) (string, error) {
+			art := filepath.Join(dir, "h.hfbc")
+			if out, err := exec.Command(bin, "-compile-hfir-bc", src, "-o", art).CombinedOutput(); err != nil {
+				t.Fatalf("hfir compile: %v %s", err, out)
+			}
+			return runBC(bin, dir, art)
+		}},
+	}
+}
+
+func runHeldoutProgram(t *testing.T, bin, program, want string, after func(t *testing.T, dir string)) {
+	t.Helper()
+	for _, p := range heldoutPaths() {
+		t.Run(p.name, func(t *testing.T) {
+			dir := t.TempDir()
+			src := filepath.Join(dir, "p.howl")
+			body := strings.ReplaceAll(program, "$DIR", dir)
+			if err := os.WriteFile(src, []byte(body), 0600); err != nil {
+				t.Fatal(err)
+			}
+			out, err := p.run(t, bin, dir, src)
+			if err != nil {
+				t.Fatalf("run: %v %s", err, out)
+			}
+			if out != want {
+				t.Fatalf("got %q want %q", out, want)
+			}
+			if after != nil {
+				after(t, dir)
+			}
+		})
+	}
+}
+
+// Bug 1: shorthand nested inside value-taking built-in arguments expands under
+// the governed profile.
+func TestHeldoutNestedShorthandExpands(t *testing.T) {
+	bin := c1c2Binary(t)
+	program := `(do
+ (let (t "a foo b")
+  (do
+   (write_file "$DIR/out.txt" (str_replace t "foo" "bar"))
+   (print (to_lower (str "AB" "C")) (read_text "$DIR/out.txt")))))`
+	runHeldoutProgram(t, bin, program, "abc a bar b\n", nil)
+}
+
+// Bug 1, fail-closed half: a forbidden construct hidden in a nested shorthand
+// argument is rejected by the governed build; no artifact, no effect.
+func TestHeldoutNestedShorthandGovernedFailsClosed(t *testing.T) {
+	bin := c1c2Binary(t)
+	dir := t.TempDir()
+	side := filepath.Join(dir, "side")
+	src := filepath.Join(dir, "x.howl")
+	art := filepath.Join(dir, "x.hfbc")
+	program := `(do (write_file "` + side + `" "ran") (write_file "` + side + `2" (str_replace (exec "touch" "` + side + `.exec") "a" "b")))`
+	if err := os.WriteFile(src, []byte(program), 0600); err != nil {
+		t.Fatal(err)
+	}
+	if out, err := exec.Command(bin, "build", "--profile", "governed", src, "-o", art).CombinedOutput(); err == nil {
+		t.Fatalf("governed build accepted nested exec: %s", out)
+	}
+	for _, p := range []string{art, side, side + "2", side + ".exec"} {
+		if _, err := os.Stat(p); err == nil {
+			t.Fatalf("%s exists after a rejected governed build", p)
+		}
+	}
+}
+
+// Bug 2: and/or short-circuit. The right side is not evaluated when the left
+// decides, so its effects (write_file) and its errors (to_int "qq") never run.
+func TestHeldoutShortCircuitSkipsRightSide(t *testing.T) {
+	bin := c1c2Binary(t)
+	program := `(do
+ (print (and (= 1 2) (do (write_file "$DIR/and.txt" "ran") (= 1 1)))
+        (or (= 1 1) (do (write_file "$DIR/or.txt" "ran") (= 1 1)))
+        (and (= 1 2) (= (to_int "qq") 1))
+        (or (= 1 1) (= (to_int "zz") 1)))
+ (print (and (= 1 1) (do (write_file "$DIR/and_rhs.txt" "ran") (= 2 2)))
+        (or (= 1 2) (= 2 3))))`
+	runHeldoutProgram(t, bin, program, "false true false true\ntrue false\n", func(t *testing.T, dir string) {
+		for _, name := range []string{"and.txt", "or.txt"} {
+			if _, err := os.Stat(filepath.Join(dir, name)); err == nil {
+				t.Fatalf("short-circuited right side ran: %s exists", name)
+			}
+		}
+		if _, err := os.Stat(filepath.Join(dir, "and_rhs.txt")); err != nil {
+			t.Fatalf("undecided right side did not run: %v", err)
+		}
+	})
+}
+
+// Bug 3: map_get of a missing key is nil, so is_nil and nz defaults work. nil
+// renders as "" in text.
+func TestHeldoutMapGetMissIsNil(t *testing.T) {
+	bin := c1c2Binary(t)
+	program := `(do
+ (let (m (dict ("a" "x") ("e" "")))
+  (do
+   (print (is_nil (map_get m "b")) (is_nil (map_get m "e")) (= (map_get m "b") ""))
+   (print (nz (map_get m "b") "dflt") (nz (map_get m "a") "dflt"))
+   (print (str "[" (map_get m "b") "]") (map_get m "b")))))`
+	runHeldoutProgram(t, bin, program, "true false false\ndflt x\n[] \n", nil)
+}
+
+// Bug 4: lists are values. append rebinds only the named variable; a list held
+// in a map (or another variable) is unchanged until it is map_set back.
+func TestHeldoutListsAreValues(t *testing.T) {
+	bin := c1c2Binary(t)
+	program := `(cli_app
+  (let (m (dict ("xs" (list "a"))))
+    (let (xs (map_get m "xs"))
+      (let (ys xs)
+        (do
+          (append xs "b")
+          (print "map" (list_len (map_get m "xs")))
+          (print "alias" (list_len ys))
+          (print "var" (list_len xs))
+          (map_set m "xs" xs)
+          (print "after" (list_len (map_get m "xs")))
+          (print (str_join (map_get m "xs") ","))
+        )))))`
+	runHeldoutProgram(t, bin, program, "map 1\nalias 1\nvar 2\nafter 2\na,b\n", nil)
+}