Skip to content

fix(interp): eval_decl handles FnExtern (#328 build-failure root cause) - #346

Merged
hyperpolymath merged 1 commit into
mainfrom
claude/fix-interp-eval-decl-fnextern
May 24, 2026
Merged

fix(interp): eval_decl handles FnExtern (#328 build-failure root cause)#346
hyperpolymath merged 1 commit into
mainfrom
claude/fix-interp-eval-decl-fnextern

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

Summary

Unbreaks main's dune runtest gate — root cause of the 2-test failure baseline that's been red since #334 merged.

Root cause

eval_decl's TopFn fd arm matched fd.fd_body against only FnBlock and FnExpr, leaving FnExtern unmatched. Every prior interp test imported externs via use effects::{…} so this case never fired — the bug was latent.

The STDLIB-04a (#328) hermetic tests are the first to include an inline extern fn make_ref<T>(x: T) -> Ref<T> / Mut; declaration in the source they hand to Interp.eval_program. That triggers the missing arm and raises:

File "lib/interp.ml", line 1010, characters 16-21: Pattern matching failed
  Raised at Affinescript__Interp.eval_decl

Reproduces the 2 failures! in 0.088s. 298 tests run. line from every PR's build log since #334.

Fix

Wrap the body lowering in an outer match fd.fd_body with:

  • FnExternOk env (externs are runtime-bound via create_initial_env's VBuiltin table — panic / error / make_ref / get / set / etc., not via the AST).
  • FnBlock _ | FnExpr _ → existing closure construction.

The inner inner-match's FnExtern arm is assert false with a comment — it's unreachable per the outer guard and the assert makes that explicit for the next reader.

Verification

The two failing tests (#328 make_ref/set/get round-trip (Int) and (String)) now have a defined evaluation path. The third 04a test (Deno codegen __cell shape) already passed — it bypasses interp.

Test plan

  • CI build job: 2 failures!298 tests run clean
  • CI lint: should also clear (was likely propagating from the same root)
  • Hypatia DOC-FORMAT: no .md introduced

Refs #328.


Generated by Claude Code

The `TopFn fd` arm of `eval_decl` matched `fd.fd_body` against only
`FnBlock` and `FnExpr`, leaving `FnExtern` unmatched. Every prior interp
test imported externs via `use effects::{…}` so the case never fired;
the bug was latent.

The STDLIB-04a (#328) hermetic tests are the first to include an inline
`extern fn make_ref<T>(x: T) -> Ref<T> / Mut;` declaration in the source
they hand to `Interp.eval_program`. That triggers the missing arm and
raises "Pattern matching failed" at `lib/interp.ml:1010`, which is the
"2 failures!" reported by `dune runtest` on every PR since #334.

Fix: wrap the body lowering in an outer `match fd.fd_body with`. The
`FnExtern` arm returns `Ok env` (externs are runtime-bound via
`create_initial_env`'s `VBuiltin` table — `panic`/`error`/`make_ref`/
`get`/`set`/etc. — not via the AST). The inner `FnBlock|FnExpr` arm
keeps the existing closure construction; an `FnExtern` fall-through is
unreachable per the outer guard and asserted to make that explicit to
the reader.

This unbreaks main: the 2 failing tests
(`#328 make_ref/set/get round-trip (Int)` and `(String)`) now have a
defined evaluation path. The codegen-only test of the same trio
already passed (it bypasses interp).

Refs #328.
@github-actions

Copy link
Copy Markdown

🔍 Hypatia Security Scan

Findings: 133 issues detected

Severity Count
🔴 Critical 15
🟠 High 62
🟡 Medium 56

⚠️ Action Required: Critical security issues found!

View findings
[
  {
    "reason": "Stray AI.a2ml in root -- use 0-AI-MANIFEST.a2ml only",
    "type": "banned",
    "file": "AI.a2ml",
    "action": "delete",
    "rule_module": "root_hygiene",
    "severity": "high"
  },
  {
    "reason": "Superseded by 0-AI-MANIFEST.a2ml",
    "type": "banned",
    "file": "AI.djot",
    "action": "delete",
    "rule_module": "root_hygiene",
    "severity": "high"
  },
  {
    "reason": "Issue in quality.yml",
    "type": "missing_workflow",
    "file": "quality.yml",
    "action": "create",
    "rule_module": "workflow_audit",
    "severity": "high"
  },
  {
    "reason": "Issue in security-policy.yml",
    "type": "missing_workflow",
    "file": "security-policy.yml",
    "action": "create",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Action hyperpolymath/standards/.github/workflows/governance-reusable.yml@main needs attention",
    "type": "unpinned_action",
    "file": "governance.yml",
    "action": "pin_sha",
    "rule_module": "workflow_audit",
    "severity": "high"
  },
  {
    "reason": "Action actions/checkout@v6 needs attention",
    "type": "unpinned_action",
    "file": "publish-jsr.yml",
    "action": "pin_sha",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Action denoland/setup-deno@v2 needs attention",
    "type": "unpinned_action",
    "file": "publish-jsr.yml",
    "action": "pin_sha",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "TypeScript file detected -- banned language",
    "type": "banned_language_file",
    "file": "/home/runner/work/affinescript/affinescript/affinescript-deno-test/example/smoke_driver.ts",
    "action": "flag",
    "rule_module": "cicd_rules",
    "severity": "critical"
  },
  {
    "reason": "TypeScript file detected -- banned language",
    "type": "banned_language_file",
    "file": "/home/runner/work/affinescript/affinescript/affinescript-deno-test/cli.ts",
    "action": "flag",
    "rule_module": "cicd_rules",
    "severity": "critical"
  },
  {
    "reason": "TypeScript file detected -- banned language",
    "type": "banned_language_file",
    "file": "/home/runner/work/affinescript/affinescript/affinescript-deno-test/mod.ts",
    "action": "flag",
    "rule_module": "cicd_rules",
    "severity": "critical"
  }
]

Powered by Hypatia Neurosymbolic CI/CD Intelligence

@hyperpolymath
hyperpolymath merged commit 660519e into main May 24, 2026
16 of 20 checks passed
@hyperpolymath
hyperpolymath deleted the claude/fix-interp-eval-decl-fnextern branch May 24, 2026 16:27

Copy link
Copy Markdown
Owner Author

Follow-up queued: broader "inline extern" fixture

This PR's fix is correct and scoped. Queueing a small follow-up for after this lands:

Add a test/e2e/fixtures/inline_extern_*.affine set covering the class of "inline extern fn declaration fed to every downstream consumer", not just the specific make_ref / error<T> paths that the STDLIB-04 tests now exercise.

The point of this PR is the missing FnExtern arm in Interp.eval_decl — survived since the interpreter was written because no test ever fed an inline extern fn to Interp.eval_program. STDLIB-04a was the first; PR #346 is the fix. To make sure the next latent gap of this shape (e.g. an extern with an effect row, a polymorphic extern with multiple type params, an extern type in a module that other modules import) surfaces against the gate rather than against the next agent, a small fixture set keyed on declaration shape — not on any one stdlib's externs — is worth ~30 lines of test code.

Suggested coverage:

  • inline_extern_pure.affineextern fn f(x: Int) -> Int; (no effects)
  • inline_extern_effectful.affineextern fn f(x: Int) -> Int / { IO };
  • inline_extern_polymorphic.affineextern fn id[T](x: T) -> T;
  • inline_extern_type_consumed.affineextern type Handle; extern fn use(h: Handle) -> Int;

Each fed through parse → resolve → typecheck → interp → at least one codegen target. The PR for this should Refs #346 and Refs the next CLAUDE.md "Test-fixture hygiene for latent bug surfaces" note (see claude/coordinator-feedback branch).

Not blocking on this PR; just don't want the fixture pattern lost in the merge.


Generated by Claude Code

hyperpolymath added a commit that referenced this pull request May 24, 2026
#348)

…erience

Captures five operational findings from the parallel-claude
coordinator-feedback turn (2026-05-24), as a single "Agent operations
notes" section:

* CI signal reliability — "PR merged" does NOT mean "build green" on
this repo today; auto-merge fires through red builds. Recently-merged
PRs (#334/#335/#336/#344) all landed with `build` red; the red persisted
until PR #346.

* Reading CI logs — WebFetch on the Actions UI returns React skeleton;
use mcp__github__pull_request_read get_check_runs / get_status, hand
back to user for actual log lines via `gh run view --log-failed`.

* Known-failing baseline checks — vscode-smoke (npm 404),
migration-assistant (fixed by #342 but stale-base branches red),
governance/Language anti-pattern policy (flags approved TS exemptions),
Hypatia 143-finding comment (mostly the same exemption hits). Don't
waste turns investigating per-PR; only investigate *changes* in this
set.

* Branching discipline — `git fetch origin main && git rebase
origin/main` immediately before push, not just at branch- creation.
Claude 1's #337 accidentally reverted #334+#335 from a stale base; cheap
to prevent.

* Test-fixture hygiene — when adding a new declaration shape (extern fn,
etc.), test it against EVERY downstream consumer
(parse/resolve/typecheck/interp/codegen). PR #346's FnExtern interp bug
survived since the interpreter was written because no test fed an inline
`extern fn` to Interp.eval_program.

Pure documentation. Zero behavioural risk.

Refs PR #346, #335, #337, #344

---------

Co-authored-by: Claude <noreply@anthropic.com>
hyperpolymath added a commit that referenced this pull request May 24, 2026
#349)

…gate)

Two `print(*r)` example fragments inside doc-comment blocks (lines 1115
and 1338) contained the literal substring `(*r`, which OCaml's lexer
treats as a nested comment opener — bringing the outer comment to depth
2 with only one matching `*)` to close it. Result: "Comment not
terminated" at EOF on every `dune build`. Marker count was 126 `(*` vs
124 `*)` (off by 2, one per offending example); after this patch it is
124 / 124.

This has blocked main's `dune build` since #335 (CORE-01 pt3 Slice A)
merged, which is why every subsequent PR (#341, #344, the open #346) has
shown `build` red regardless of its own changes. Tiny fix; high
leverage.

Fix: insert a single space inside the parens — `print( *r)` — so the
lexer sees `(` `*` `r` `)` rather than the comment-open token `(*`
followed by `r`. Reader-friendly; the example still reads the same.

No behavioural change. No code touched. Comment text only.

Diagnosed by parallel claude (INT-03 / S5 thread). Refs #335

Co-authored-by: Claude <noreply@anthropic.com>
hyperpolymath added a commit that referenced this pull request May 24, 2026
## Summary

Adds **class-level coverage** for the "inline extern fed to every
downstream consumer" surface that produced the PR #346 `FnExtern` interp
bug (`Interp.eval_decl`'s `TopFn` arm missing the `FnExtern` match arm —
silent pattern-match failure since the interpreter was written; fired
the moment STDLIB-04a's tests became the first to hand an inline `extern
fn` to `Interp.eval_program`).

Queued as a [comment on PR
#346](#346 (comment))
in the previous session; this is the follow-up.

## What this PR adds

Four fixtures under `test/e2e/fixtures/`:

| Fixture | Shape |
|---------|-------|
| `inline_extern_pure.affine` | `extern fn host_pure_identity(x: Int) ->
Int;` (no effects) |
| `inline_extern_effectful.affine` | `extern fn host_log(msg: String) ->
Unit / IO;` (effect row) |
| `inline_extern_polymorphic.affine` | `extern fn host_identity[T](x: T)
-> T;` (type params) |
| `inline_extern_type_consumed.affine` | `extern type Handle; extern fn
host_use(h: Handle) -> Int;` |

Each fed through **parse → resolve → typecheck → interp** via the new
`inline_extern_pipeline_ok` helper in `test/test_e2e.ml`. Assertion: all
four phases return `Ok`. A regression that re-introduces the silent
pattern-match-failure path that broke main between #334 and #346 would
fail loudly here instead.

Suite registered as `"E2E Inline Extern Shapes (Refs #346)"`.

## Why this matters

Per the `.claude/CLAUDE.md` §"Test-fixture hygiene for latent bug
surfaces" rule landed this session: when adding a stdlib `extern fn` (or
any other new declaration shape), test it against **every downstream
consumer** (parse / resolve / typecheck / interp / codegen). PR #346
fixed *one* instance of this class; this PR pins the *class* against the
gate so the next latent gap of the same shape (an `extern type` consumed
by an `extern fn` in a module that other modules import, etc.) surfaces
against CI rather than against the next agent.

## Test plan

- [ ] CI `build` job clean
- [ ] CI `dune runtest` — four new alcotest cases pass; existing suite
unchanged
- [ ] No new lints

Refs #346, Refs `.claude/CLAUDE.md` §"Test-fixture hygiene for latent
bug surfaces"

---
_Generated by [Claude
Code](https://claude.ai/code/session_01WHUYQEPKgQU6jBgUj4snYU)_

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants