issue fix

[navicore]

Summary of the fix:

#424

• New runtime FFI patch_seq_test_set_name in crates/runtime/src/test.rs — takes a String off the stack, updates only ctx.current_test, does NOT clear failures / passes / current_line. Re-exported from runtime/src/lib.rs.
• New Seq builtin test.set-name with signature ( ..a String -- ..a ) registered in builtins/diagnostics.rs, in the codegen/runtime/test_time.rs DECLS + SYMBOLS tables, and in the ast/program.rs word-call validator's builtin list.
• Runner wrapper in test_runner.rs now emits "{name}" test.init {name} "{name}" test.set-name test.finish, which guarantees the test.finish header matches the word name regardless of whether the user called test.init with a different friendly name inside their test word.

No user-visible API change — test.set-name is documented as a
runner-internal helper but exists on the Seq surface (it's called from
the generated wrapper, which is compiled as normal Seq). test.init still
accepts and honors a user-supplied friendly name for standalone test
files (no runner wrapper).

[claude]

Code Review

Overview

This PR fixes a test output attribution bug where a user calling test.init "friendly name" inside their test word would overwrite ctx.current_test, causing test.finish to print a header that didn't match the word name collect_failure_block was keying on — orphaning failure detail lines in the runner output.

The fix adds a minimal test.set-name builtin (runtime FFI + Seq surface) and has the runner wrapper emit it between the test word call and test.finish, reasserting the canonical word name.

---

What's Working Well

• Surgical scope. patch_seq_test_set_name touches only ctx.current_test and nothing else — no accidental state reset. The contrast with ctx.reset(...) in test.init is intentional and correct.
• Registration is complete. All four required sites are updated: ast/program.rs (word-call validator), builtins/diagnostics.rs (type signature + docs), codegen/runtime/test_time.rs (DECLS + SYMBOLS), and runtime/lib.rs (re-export). Easy to miss one; didn't.
• Wrapper template is correct. "{0}" test.init {0} "{0}" test.set-name test.finish sequences cleanly — test.init initialises state, the word runs (possibly calling its own test.init), then test.set-name reasserts the authoritative name before test.finish reads it.
• Implementation mirrors existing patterns. String extraction (Value::String(s) => s.as_str().to_string()), the double-unsafe block, and the doc-comment style all match patch_seq_test_init exactly — consistent with the existing codebase conventions.
• DECLS category placement is correct: category: None for subsequent entries in a group (only the first gets the section label comment), matching patch_seq_test_finish.

---

Issues & Suggestions

Minor: doc comment stack-effect inconsistency

patch_seq_test_init documents its effect as ( name -- ) while the new patch_seq_test_set_name uses the richer ( ..a String -- ..a ) form. The new form is strictly more informative (preserves the rest-of-stack explicitly). Worth updating patch_seq_test_init's doc comment to match for consistency — but that's a follow-up nit, not a blocker.

Suggestion: add a unit test for patch_seq_test_set_name

There are no new unit tests covering the Rust implementation directly. The existing mod tests in test_runner.rs likely exercises the fix indirectly, but a focused unit test in test.rs would prevent future regressions and document the invariant explicitly. Something like:

#[test]
fn set_name_does_not_clear_state() {
    // set up a context with assertions already recorded
    let mut ctx = TEST_CONTEXT.lock().unwrap();
    ctx.reset(Some("original".to_string()));
    ctx.passes = 3;
    ctx.failures.push(TestFailure { line: None, message: "x".into(), expected: None, actual: None });
    drop(ctx);

    // call set_name
    let stack = /* push "new-name" */;
    unsafe { patch_seq_test_set_name(stack); }

    let ctx = TEST_CONTEXT.lock().unwrap();
    assert_eq!(ctx.current_test.as_deref(), Some("new-name"));
    assert_eq!(ctx.passes, 3);           // not cleared
    assert_eq!(ctx.failures.len(), 1);   // not cleared
}

Observation: parallel test execution

TEST_CONTEXT is a global Mutex<TestContext>. Sequential test execution (current behaviour) is safe. If seqc test ever gains parallel execution, all of these FFI functions will need a rethink — the mutex won't prevent interleaving across concurrent test runs. This isn't introduced by this PR, just worth noting as a future constraint.

---

Verdict

The fix is correct, minimal, and well-reasoned. The PR description thoroughly explains the root cause and the design decision to expose test.set-name on the Seq surface (runner-emitted, not user-facing). I'd suggest adding a unit test before merge, but the implementation itself is sound.