akbasic/TODO.md

# Record

**Outstanding work is in the issue tracker, not in this file:**
<https://source.starfort.tech/andrew/akbasic/issues>

This file was the implementation plan for the Go → C port of `deps/basicinterpret`, written to be
executed by AI agents rather than read for inspiration. **The port is done**, so what it is now is
the record: the design decisions that are settled, the deviations from the reference and why each
was taken, the defects that were found and fixed, and the reasoning behind the measurements.

Each open item that moved leaves a line saying what it was and which issue carries it, because an
entry explaining *why* a defect matters is worth keeping beside the work it constrains — but the
tracking happens there, not here.

Issues are labelled by kind and blast radius, and milestoned by what they can land in: `0.1.x` for
anything that changes no public contract, `0.2.0` for new verbs and observable behaviour changes,
`1.0.0` for the design decisions. Everything filed carries `status::grooming`.

Two gaps this file recorded and deliberately did not file — the missing `HUD` anchors and a
dismissable dialog — are now `libakgl` #79 and #80. §7's rule is right that *changing* a dependency
is that repository's decision; it does not follow that reporting the gap is.

---

## 0. Agent protocol

### 0.1 The Go reference is deprecated. Stop matching it.

**`deps/basicinterpret` is a dead project. It will not be updated, and this interpreter is no
longer required to reproduce its behaviour.** Recorded here first because it silently reverses
the premise several sections of this file were written on, and because an agent that reads them
without this will park work that is no longer blocked.

What it changes:

- **§6 is now an ordinary defect list.** "Port the behaviour first so the port is provably
  faithful" is retired. Fix them because they are wrong, not when fidelity permits.
- **§1.8's message-text contract is now a convention.** Improving a message is allowed; it costs
  a golden file, which is a cost rather than a veto.
- **§5's bar drops** from "defensible against the golden suite" to defensible on its own merits.
- **`tests/language/` is the editable language corpus rather than a protected specification.** Diverging from
  it is allowed and must be deliberate and recorded — see its README.

What it does **not** change:

- The corpus stays and stays green. Forty-one real BASIC programs with known-good output are
  worth having whatever their provenance, and an unexplained change there is still a red flag.
- The Go source stays readable as documentation. It remains the best answer to "what did the
  original actually do here", which is a question worth being able to answer even once the
  answer stops being binding.
- Nothing about the `ak*` house rules, which never came from the reference.

**Read these before touching anything**, in this order:

1. `MAINTENANCE.md` in this repository — project goals, the `libakerror` convention, the
   error-code range map, and the dependency versions.
2. `deps/libakerror/AGENTS.md` — the `ATTEMPT`/`CLEANUP`/`PROCESS`/`HANDLE`/`FINISH` protocol.
3. `deps/libakerror/UPGRADING.md` — 1.0.0's status registry. Required before writing an error
   code; the mechanism it replaced is gone.
4. `deps/libakstdlib`'s issue tracker — the defects and gaps in the library this port calls
   into. §1.9 below says which calls are cleared for use; that section is not optional
   reading, it bans a family of functions the port would otherwise reach for by reflex.
5. `deps/libakgl/AGENTS.md` — the no-`malloc` rule and the commit co-author requirement.
6. `deps/basicinterpret/README.md` — the language reference and the unimplemented list. Still
   worth reading for the verb set and the semantics; no longer binding, per §0.1.

**Rules for working this file:**

- **Do not** mark an item done until its acceptance command passes on a clean out-of-tree
  build. "It compiles" is not acceptance.
- **Do** update the tracker in the same commit as the work: close the issue, or replace it with
  the defect it uncovered. Outstanding items live there; this file holds the record.
- **Do** add the agent program name, model name and version as a commit co-author. That rule
  comes from `libakgl` and applies here.
- **Never** hand-edit generated output. `build/` trees and the generated `akerror.h` are
  off-limits; change the generator.
- **Never** reformat a file you are not otherwise changing.
- When a step is blocked because `libakgl` cannot supply a capability, **do not work around
  it here.** File it against `libakgl` in its issue tracker -- what the BASIC verb requires,
  what the `akgl_*` entry point should look like, and what tests would cover it -- and note the
  block in §7 below.

**Style**, restated so nobody has to go look: C99, 4-space indent, tabs at width 8
(`stroustrup`), function-body braces in column 0 on their own line, control braces on the same
line, **always brace**, spaces inside control-flow parens — `if ( x == y ) {`. Pointer star
binds to the identifier: `char *name`. Prefix is `akbasic_` for functions,
`akbasic_TypeName` for types, `AKBASIC_UPPER_SNAKE` for macros. `static` helpers drop the
prefix. Parameter names must match between header and source.

---

## 1. Design decisions already made

These are settled. Do not relitigate them mid-port; if evidence says one is wrong, say so in
a commit that changes it deliberately, and update this section.

### 1.1 Reflection becomes one aligned dispatch table

The Go runtime resolves verbs with `reflect.MethodByName("Command" + NAME)`
(`basicruntime.go:401`), functions with `"Function" + NAME`, and special parse paths with
`"ParseCommand" + NAME` (`basicparser.go:103`). C has no reflection and we are not adding
any. Replace all three with **one** static table in `src/verbs.c`, sorted by name, searched
with `bsearch(3)`:

```c
/* name        token type              parse handler          exec handler        */
{ "AUTO",      AKBASIC_TOK_CMDIMM,     NULL,                  cmd_auto      },
{ "DATA",      AKBASIC_TOK_COMMAND,    parse_data,            cmd_data      },
{ "DEF",       AKBASIC_TOK_COMMAND,    parse_def,             cmd_def       },
```

A `NULL` parse handler means "parse the rval as a plain expression", which is exactly what
`commandByReflection` returning `(nil, nil)` means today. Adding a verb is adding one row plus
two functions. **Keep the table column-aligned and one row per verb** — it is a table, so it
gets laid out as one.

This also kills the Go scanner's three separate maps (`reservedwords`, `commands`,
`functions`, `basicscanner.go:64-67`): the token type lives in the same row.

### 1.2 Strings are fixed-size and live inline

`libakstdlib` has no string type. `libakgl` has `akgl_String` but it is `PATH_MAX` bytes,
refcounted, and pool-allocated — wrong shape for a value that gets copied on every
assignment, and it would drag a `libakgl` dependency into the core interpreter.

Define in `include/akbasic/types.h`:

```c
#define AKBASIC_MAX_STRING_LENGTH    256    /* matches AKBASIC_MAX_LINE_LENGTH */
```

and give `akbasic_Value` a `char stringval[AKBASIC_MAX_STRING_LENGTH]` **inline**. `clone()`
becomes a struct assignment. No allocator, no refcount, no lifetime question.

**Tradeoff, stated:** every `akbasic_Value` is ~300 bytes, so one environment's
`values[AKBASIC_MAX_VALUES]` pool is ~19KB, and 32 environments is ~610KB of BSS. That is
fine on a PC and is the price of never calling `malloc`. If it ever isn't fine, the knob is
`AKBASIC_MAX_STRING_LENGTH`, not the allocator.

Truncation is an **error**, not a silent clamp: `FAIL_RETURN(e, AKBASIC_ERR_VALUE, ...)`.

### 1.3 Maps become fixed-capacity open-addressed tables

Five Go maps need replacing:

| Go site | Purpose | C replacement |
|---|---|---|
| `BasicScanner.reservedwords/commands/functions` | keyword → token type | the §1.1 static table + `bsearch` |
| `BasicEnvironment.variables` | name → `*BasicVariable` | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_VARIABLES` |
| `BasicEnvironment.functions` | name → `*BasicFunctionDef` | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_FUNCTIONS` |
| `BasicEnvironment.labels` | name → line number | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_LABELS` |

One implementation, `src/symtab.c`, keyed by `aksl_strhash_djb2()` (already in
`libakstdlib`) with linear probing and a fixed slot array. **Use the existing hash; do not
write another one.** Table full is an error, not a resize.

Caveat, recorded upstream: the wrapper sign-extends `char`, so a
high-bit byte hashes wrong — `"\xff\xfe"` returns 5859874 where the `unsigned char` answer is
5868578. BASIC identifiers are 7-bit ASCII (the scanner only accepts `IsLetter`/`IsDigit` plus
a type suffix), so this cannot bite the symbol tables. It **would** bite if anyone later keys
a table on a string literal or a filename. Do not work around it here; it is already filed
upstream.

### 1.4 Environments come from a pool and are released

Go calls `new(BasicEnvironment)` at `basicruntime.go:121` and `basicparser_commands.go:124`
and never frees one. A long-running `GOSUB` or `FOR` in Go leaks; the GC eventually catches
some of it, and nothing in the tests notices.

C gets `HEAP_ENVIRONMENT[AKBASIC_MAX_ENVIRONMENTS]` with
`akbasic_env_acquire()` / `akbasic_env_release()`, in the shape of `akgl_heap_next_*`.
`akbasic_runtime_prev_environment()` **must** release the environment it pops. Pool
exhaustion is `AKBASIC_ERR_ENVIRONMENT`, reported with the current line number.

Watch the one place this is not a clean stack: `userFunction` (`basicruntime.go:348`) stores
a `BasicEnvironment` **by value** inside `BasicFunctionDef` and re-`init()`s it on every call.
In C the funcdef holds an `akbasic_Environment *` acquired at `DEF` time and reset per call —
it is owned by the funcdef, not the pool's free list, until the funcdef dies.

### 1.5 Output goes through a text sink backend

`Write()` and `Println()` (`basicruntime_graphics.go:140,148`) mirror every line to stdout
*and* to an SDL surface. That mirror is the only reason the golden-file suite works. Do not
reproduce it as a hardcoded pair of calls.

Define a record of function pointers, populated by an initializer — the house pattern:

```c
typedef struct akbasic_TextSink
{
    void *self;
    akerr_ErrorContext AKERR_NOIGNORE *(*write)(struct akbasic_TextSink *self, char *text);
    akerr_ErrorContext AKERR_NOIGNORE *(*writeln)(struct akbasic_TextSink *self, char *text);
    akerr_ErrorContext AKERR_NOIGNORE *(*readline)(struct akbasic_TextSink *self, char *dest, size_t len);
    akerr_ErrorContext AKERR_NOIGNORE *(*clear)(struct akbasic_TextSink *self);
} akbasic_TextSink;
```

`akbasic_sink_init_stdio()` is in the library and `akbasic_sink_init_akgl()` is in the
akgl-backed module. The driver is supposed to pick: a default build selects stdio, while an
`AKBASIC_WITH_AKGL` build selects the SDL text path and mirrors its output to stdout. It does
not do that yet; §3 records the missing standalone frontend. Cursor arithmetic, wrapping and
scrolling belong to the akgl sink, not to the interpreter.

### 1.6 The interpreter steps; it does not run

Go's `run()` (`basicruntime.go:682`) is a `for {}` that owns the process until `MODE_QUIT`.
Goal 3 forbids that: a host game must be able to bound execution.

The library exposes:

```c
akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_step(akbasic_Runtime *obj);
akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_run(akbasic_Runtime *obj, int maxsteps);
```

`_step()` does exactly what one iteration of Go's `for {}` body does and returns.
`_run(obj, maxsteps)` loops `_step()` until the mode is `AKBASIC_MODE_QUIT` or `maxsteps`
steps have elapsed; `maxsteps <= 0` means unbounded, which is what the standalone driver
passes. **This is a deliberate restructure, and it must not change a single byte of golden
output.**

`QUIT` sets `AKBASIC_MODE_QUIT` and returns. **Nothing in the library calls `exit()`,
`abort()`, or `FINISH_NORETURN`.** `FINISH_NORETURN` appears exactly once in the tree, in
`src/main.c`.

### 1.7 Error codes are absolute, reserved from the 1.0.0 registry

`deps/libakerror` is at **1.0.0**. Read `deps/libakerror/UPGRADING.md` before writing an error
code; `MAINTENANCE.md`'s error-code section is the condensed version. Three things this changes
from what you may have seen in an older draft or in `libakgl`:

- `AKERR_MAX_ERR_VALUE` **no longer exists**. The name registry is sparse and takes any `int`.
  There is nothing for a consumer to size, and no compile definition to set. Anywhere you find
  one, delete it.
- `__AKERR_ERROR_NAMES` **no longer exists**. The table is private.
- Codes are **absolute integer constants at 256 or above**, never `AKERR_LAST_ERRNO_VALUE + N`.
  The offset scheme is what produced the live `libakgl` collision documented in
  `MAINTENANCE.md`.

`akbasic` owns **512–767** per the range map in `MAINTENANCE.md`. Declare it as an `enum`, so the
values stay compile-time integer constants (`HANDLE` expands to `case` labels, which require
that) and adding a code does not mean renumbering an offset:

```c
#define AKBASIC_OWNER "akbasic"

enum {
    AKBASIC_ERR_BASE        = 512,   /** Start of akbasic's reserved status range */
    AKBASIC_ERR_SYNTAX      = AKBASIC_ERR_BASE,
                                     /** Parse-time grammar violation */
    AKBASIC_ERR_TYPE,                /** Incompatible types in an operation */
    AKBASIC_ERR_UNDEFINED,           /** Reference to an undefined verb, function or label */
    AKBASIC_ERR_BOUNDS,              /** Array subscript or pool index out of range */
    AKBASIC_ERR_ENVIRONMENT,         /** Environment pool exhausted or orphaned environment */
    AKBASIC_ERR_VALUE,               /** A value was malformed, truncated or unconvertible */
    AKBASIC_ERR_STATE,               /** A verb ran outside the block structure it requires */
    AKBASIC_ERR_LIMIT       = AKBASIC_ERR_BASE + 256
};
```

`akbasic_init()` reserves the **whole** 256 in one call and then names each code. Both
registry calls return `akerr_ErrorContext *` and are `AKERR_NOIGNORE`, so a collision is an
ordinary error — `PASS` it and let it propagate out of init:

```c
akerr_ErrorContext AKERR_NOIGNORE *akbasic_init(void)
{
    PREPARE_ERROR(errctx);

    PASS(errctx, akerr_reserve_status_range(AKBASIC_ERR_BASE,
					    AKBASIC_ERR_LIMIT - AKBASIC_ERR_BASE,
					    AKBASIC_OWNER));
    PASS(errctx, akerr_register_status_name(AKBASIC_OWNER, AKBASIC_ERR_SYNTAX,
					    "Syntax Error"));
    /* ... one per code ... */
    SUCCEED_RETURN(errctx);
}
```

Reserve the whole range in **one** call — a subset or superset of your own range raises
`AKERR_STATUS_RANGE_OVERLAP`, not a no-op. Use `akerr_register_status_name()`, never the
two-argument `akerr_name_for_status(status, name)` set path: the owned form is the one that
catches a component writing into a range that is not its own, and the two-argument form is
what let `libakgl` silently clobber `libakerror`'s names.

There is no startup ceiling to assert any more — the BSS-overflow hazard the old draft
defended against was deleted along with `AKERR_MAX_ERR_VALUE`. What replaces it is the
reservation itself: if `akbasic_init()` returns an error, something else owns part of 512–767
and the process must not continue as though it does not.

Do not call `akerr_init()` first. Every registry entry point calls it, and since 1.0.0 it no
longer clears reservations made before it ran.