diff --git a/jac/jaclang/byllm/tests/TESTING.md b/jac/jaclang/byllm/tests/TESTING.md new file mode 100644 index 00000000000..bbf44a7605d --- /dev/null +++ b/jac/jaclang/byllm/tests/TESTING.md @@ -0,0 +1,199 @@ +# Testing byLLM + +Read this before adding a test here. It is short on purpose. + +The suite is mid-refactor (jaseci-labs/jac#9002). New tests follow the rules below; +old tests that do not are being converted cluster by cluster. `test_guard.jac` holds +today's counts and fails a PR that moves one the wrong way, so you find out here rather +than in review. + +## Run it + +``` +JAC_TEST_JOBS=2 JAC_TEST_STRICT=1 jac test jac/jaclang/byllm/tests \ + --ignore jac/jaclang/byllm/tests/test_mtir_integration.jac +``` + +`test_mtir_integration.jac` runs in the sealed lane, from a copy outside the checkout; +`ci.yml` explains why. Read the pass count, not the exit code. + +## The one rule + +**Fake the model at `model_call_*`, never above it.** + +A `by llm()` call goes through these layers: + +``` + your function + MTRuntime: messages, resp_type, tools + BaseLLM.invoke: react loop, retries, compaction + dispatch_no_streaming + make_model_params ......... prompt, semstrings, schema, tool descriptions + model_call_* .............. the network hop <-- fake HERE + _parse_tool_calls, parse_response .................. typed output +``` + +`FakeLLM` replaces only the network hop, so everything above and below it is the real +code. That buys two things a higher fake cannot give you: the request is really built, +so you can assert on it, and the reply is really parsed, so a declared return type is +actually enforced. + +`MockLLM` replaces `dispatch_no_streaming`, four layers up. Under it the prompt is +built and thrown away and `parse_response` never runs. Two functions with opposite +`sem` strings return identical output, and `def f() -> int by llm()` hands back a +`str`. It is still exported for users, but do not reach for it in new tests here. + +## Writing a test + +```jac +import from support_tests { FakeLLM, say, call, finish, mk_run } + +glob emoji_llm = FakeLLM(replies=[say("")]); + +def get_emoji(text: str) -> str by emoji_llm(temperature=0.7); + +test "temperature is passed through and the input reaches the prompt" { + assert get_emoji("lets move to paris") == ""; + + sent = emoji_llm.seen[0]; + assert sent["temperature"] == 0.7; + assert "paris" in str(sent["messages"]).lower(); +} +``` + +The last two assertions are the point. Assert on what came back **and** on what went +out. A test that only checks the return value is checking that the reply you scripted +came back, which is true by construction. + +What the annex gives you, all in `support_tests.jac`: + +| | | +|---|---| +| `FakeLLM(replies=[...])` | a `BaseLLM` that serves scripted replies and records every outgoing params dict | +| `scripted(model, replies)` | same, but patched onto a real `Model` when you need provider behaviour | +| `say(text)` | a plain-text reply | +| `call(name, args)`, `calls([...])` | one or several tool calls | +| `finish(value)` | a `finish_tool` call | +| `fail(err, content, after)` | raise, optionally mid-stream | +| `mk_run(...)` | an `MTRuntime` with sane defaults | +| `response()`, `chunks()` | real `litellm` objects, for tests that drive dispatch directly | +| `load_fixture(name)` | import a fixture module | +| `run_fixture(name)` | import a fixture and capture its stdout (legacy; prefer values) | +| `llm.sent(key)`, `llm.seen`, `llm.exhausted()` | what was sent, and whether the script was drained | + +Add to the annex rather than defining a helper locally. If a helper already exists +under a different name, use it; if two exist, delete one. + +## Fixtures + +Most tests do not need one. A `by llm()` def belongs in the test file next to its test. + +A fixture is for a **program whose shape is the subject**, and it contains no model, no +entry block, no prints and no asserts. Only two kinds qualify: + +- **graph programs**: `node` / `walker` / `edge` with `visit ... by llm()` +- **compile targets**: files the MTIR, import and scope tests compile or analyze + +```jac +# fixtures/routing_graph.jac -- the program under test, nothing else +glob llm: any = None; # the test assigns its own fake + +edge Route { has priority: str = "low"; } +node Desk {} +node Agent { + has tag: str = "x"; + can handle with dispatcher entry { visitor.visited.append(self.tag); } +} +walker dispatcher { + has visited: list = []; + can route with Desk entry { visit [-->] by llm(select=1); } +} +``` + +```jac +# the test drives it +import from fixtures.routing_graph { Desk, Agent, Route, dispatcher } +import fixtures.routing_graph as rg; +import from support_tests { FakeLLM, say } + +test "routes along the edge attribute, not just the node" { + llm = FakeLLM(replies=[say('["Agent_beta"]')]); + rg.llm = llm; + + desk = root ++> Desk(); + desk +>: Route(priority="low") :+> Agent(tag="alpha"); + desk +>: Route(priority="high") :+> Agent(tag="beta"); + + w = dispatcher() spawn desk; + + assert w.visited == ["beta"]; + assert "priority='high'" in str(llm.sent("messages")[0]); +} +``` + +Five things about that example are load-bearing and were each found the hard way: + +1. **`glob llm: any = None;`**, not `glob llm = None;`. The bare form infers `NoneType` + and the test's assignment fails `jac check` with E1001. +2. **`by llm()` resolves the global at call time**, which is why the test can own the + fake and the fixture can stay pure. +3. **Static import of the archetypes.** `load_fixture()` returns the module as `any`, + and `root ++> g.Desk()` then fails `jac check` with + `E1097: Connection right operand must be a node instance`. Import the names directly; + use the module alias only to rebind `llm`. +4. **The routing reply is a handle, not an index.** The return type is + `list[enum[RouteChoice]]` and members are generated from the candidate set: two + `Agent` siblings give `Agent_alpha` / `Agent_beta`, a lone one gives `Agent`. An + index retries three times and then raises `OutputConversionError`. +5. **Spawn on the node you just made**, never from `root`. `root` accumulates across + tests in a file. Per-test subgraphs stay isolated as long as you do not walk from + the root, and you should not assert on `[root -->]`. + +Also: `Jac.jac_import` returns a **cached** module and does not re-execute it. State +carries over between calls, so every test must assign its own fake and must not rely on +fresh module state. + +## Assertions + +Assert on values. Do not grep stdout. + +| instead of | write | +|---|---| +| `assert "X_PASS" in stdout_value` | the fixture's own asserts, moved into the test, on returned values | +| `assert "Tool called with 12" in stdout` | `assert llm.sent("tools")[0][0]["function"]["name"] == "add"` and the tool's return value | +| slicing a dict out of a log line and `yaml.safe_load`ing it | `params = llm.seen[0]; assert "temperature" not in params` | + +A stdout match cannot tell a wrong answer from a missing `print`, it reports nothing +useful when it fails, and it cannot see the request at all. + +**Asserts inside a fixture are invisible to the runner.** 66 of this suite's assertions +still live in fixture files, guarded by 10 sentinel greps. If you delete such a fixture, +move every assert into the test first. `test_guard.jac` keeps a floor on the total count +because the test count alone will not notice. + +## Adding to the guard's budgets + +`test_guard.jac` is a ratchet, not a wall. Every number is today's measured count. + +- **Cleaning up?** Lower the ceiling or raise the floor in the same PR. That is the + intended direction and needs no explanation. +- **Genuinely need a new fixture?** Raise `FIXTURE_BUDGET` in the same PR and say why in + the description. +- **A number moved the wrong way and you did not mean it?** The failure message names + the file, the delta and the fix. + +## PR checklist + +Every PR that touches this directory carries this table: + +| | before | after | +|---|---|---| +| test blocks | | | +| table rows / cases | | | +| assert statements (test files) | | | +| assert statements (fixtures) | | | +| tests skipped | | | +| tests newly red, with issue number | | | + +Plus one line per deleted test naming the surviving row or assert, and one line per +dropped assert naming why. diff --git a/jac/jaclang/byllm/tests/test_guard.jac b/jac/jaclang/byllm/tests/test_guard.jac new file mode 100644 index 00000000000..1c6a1a17719 --- /dev/null +++ b/jac/jaclang/byllm/tests/test_guard.jac @@ -0,0 +1,300 @@ +"""Ratchet that keeps the byLLM suite from growing back the way it grew. + +Every number below is today's measured count, not a target. Ceilings may only fall and +floors may only rise; a PR that moves one the wrong way fails here with the reason and +the fix. Lowering a ceiling or raising a floor as you clean up is the point, and is a +one-line edit in the same PR. + +See TESTING.md for what the suite is converging on and why each rule exists. +""" + +import os; +import re; + +glob TESTS_DIR: str = os.path.dirname(os.path.abspath(__file__)), + FIXTURE_DIR: str = os.path.join(TESTS_DIR, "fixtures"), + ANNEX: str = "support_tests.jac"; + + +"""Every .jac under tests/, excluding fixtures/ and this file.""" +def test_files -> list[str] { + out: list[str] = []; + for name in sorted(os.listdir(TESTS_DIR)) { + if name.endswith(".jac") and name != "test_guard.jac" { + out.append(name); + } + } + return out; +} + + +"""Every fixture program, including the ones in subdirectories.""" +def fixture_files -> list[str] { + out: list[str] = []; + for (dirpath, _dirnames, filenames) in os.walk(FIXTURE_DIR) { + for name in filenames { + if name.endswith(".jac") { + full = os.path.join(dirpath, name); + out.append(os.path.relpath(full, FIXTURE_DIR)); + } + } + } + return sorted(out); +} + + +def read(path: str) -> str { + with open(path, "r", encoding="utf-8", errors="replace") as fh { + return fh.read(); + } +} + + +def read_test(name: str) -> str { + return read(os.path.join(TESTS_DIR, name)); +} + + +def read_fixture(rel: str) -> str { + return read(os.path.join(FIXTURE_DIR, rel)); +} + + +def count(pattern: str, text: str) -> int { + return len(re.findall(pattern, text)); +} + + +"""Compare measured counts to a baseline. `direction` is 'ceiling' or 'floor'.""" +def ratchet( + label: str, + measured: dict[str, int], + baseline: dict[str, int], + direction: str, + fix: str +) { + problems: list[str] = []; + for (key, limit) in baseline.items() { + got = measured.get(key, 0); + if direction == "ceiling" and got > limit { + problems.append(f" {key}: {got}, was {limit} (+{got - limit})"); + } + if direction == "floor" and got < limit { + problems.append(f" {key}: {got}, was {limit} ({got - limit})"); + } + } + for (key, got) in measured.items() { + if key not in baseline and got > 0 and direction == "ceiling" { + problems.append(f" {key}: {got}, not in the baseline at all"); + } + } + word = "rose" if direction == "ceiling" else "fell"; + assert not problems , ( + f"{label} {word}:\n" + + "\n".join(problems) + + f"\n\n{fix}\n\n" + + "If this is a deliberate cleanup, edit the baseline in test_guard.jac in " + + "the same PR. If it is new code, use the annex instead. See TESTING.md." + ); +} + + +# --------------------------------------------------------------------------- +# 1. Fixture programs. A new one has to be justified in the same PR. +# --------------------------------------------------------------------------- +glob FIXTURE_BUDGET: int = 66; + +test "fixture count does not grow" { + found = fixture_files(); + assert len(found) <= FIXTURE_BUDGET , ( + f"fixtures/ holds {len(found)} programs, budget is {FIXTURE_BUDGET}.\n\n" + + "Most tests do not need a fixture. A `by llm()` def belongs in the test " + + "file beside its test, scripted with FakeLLM. A fixture is only for a " + + "program whose SHAPE is the subject (node/walker/edge) or a compile " + + "target for the MTIR, import and scope tests.\n\n" + + "If this one really is a program under test, raise FIXTURE_BUDGET here in " + + "the same PR and say why in the PR description. See TESTING.md." + ); +} + + +# --------------------------------------------------------------------------- +# 2. A fixture is a program under test, not a test. +# Today 52 fixtures still print or assert; that number may only fall. +# --------------------------------------------------------------------------- +glob IMPURE_FIXTURE_BUDGET: int = 52; + +test "no new fixture asserts or prints its own result" { + impure: list[str] = []; + for rel in fixture_files() { + src = read_fixture(rel); + if count(r"(?m)^\s*assert\s", src) > 0 or count(r"print\s*\(", src) > 0 { + impure.append(rel); + } + } + assert len(impure) <= IMPURE_FIXTURE_BUDGET , ( + f"{len(impure)} fixtures assert or print, budget is " + + f"{IMPURE_FIXTURE_BUDGET}.\n\n" + + "A fixture that asserts hides its checks from the runner: the test greps a " + + "sentinel out of stdout and cannot report which assert failed. Put the " + + "assertions in the test and let it read values back directly.\n\n" + + f"Fixtures currently doing this: {len(impure)}. Lower the budget as you " + + "convert them." + ); +} + + +# --------------------------------------------------------------------------- +# 3. One way to fake the model. Every count here is a ceiling. +# --------------------------------------------------------------------------- +glob MOCKLLM_BASELINE: dict[str, int] = { + "test_compaction.jac": 14, + "test_usage.jac": 5 + }, + FIXTURE_MOCKLLM_BASELINE: int = 53, + SIMPLENAMESPACE_BASELINE: dict[str, int] = { + "test_byllm.jac": 10, + "test_compaction.jac": 4, + "test_telemetry_.jac": 8, + "test_tool_arg_streaming.jac": 7, + "test_usage.jac": 19 + }; + +test "MockLLM constructions in test files do not grow" { + measured: dict[str, int] = {}; + for name in test_files() { + if name == ANNEX { + continue; + } + n = count(r"MockLLM\s*\(", read_test(name)); + if n > 0 { + measured[name] = n; + } + } + ratchet( + "MockLLM constructions", + measured, + MOCKLLM_BASELINE, + "ceiling", + "Use FakeLLM from support_tests instead. MockLLM replaces " + + "dispatch_no_streaming, which sits above make_model_params and " + + "parse_response, so the prompt is built and discarded and the reply is " + + "never parsed: the test cannot assert on what was sent, and a typed " + + "return is not converted. FakeLLM replaces only model_call_*, so the " + + "real dispatch runs and llm.sent(...) shows the outgoing params." + ); +} + +test "fixtures reaching for MockLLM do not grow" { + n = 0; + for rel in fixture_files() { + if "MockLLM" in read_fixture(rel) { + n += 1; + } + } + assert n <= FIXTURE_MOCKLLM_BASELINE , ( + f"{n} fixtures construct a MockLLM, budget is " + + f"{FIXTURE_MOCKLLM_BASELINE}.\n\n" + + "A fixture should not own a model at all. Declare `glob llm: any = None;` " + + "and let the test assign its own FakeLLM: `by llm()` resolves the global " + + "at call time, so the fixture stays a pure program under test." + ); +} + +test "hand-rolled SimpleNamespace fakes do not grow" { + measured: dict[str, int] = {}; + for name in test_files() { + n = count(r"SimpleNamespace\s*\(", read_test(name)); + if n > 0 { + measured[name] = n; + } + } + ratchet( + "SimpleNamespace fakes", + measured, + SIMPLENAMESPACE_BASELINE, + "ceiling", + "support_tests gives you response(), chunks() and mk_run(), which build " + + "real litellm objects and a real MTRuntime. A SimpleNamespace stand-in " + + "accepts any shape, so it keeps passing after the real object's shape " + + "changes." + ); +} + + +# --------------------------------------------------------------------------- +# 4. Tests read values, not stdout. +# --------------------------------------------------------------------------- +glob STDOUT_GREP_BASELINE: dict[str, int] = { + "support_tests.jac": 2, + "test_byllm.jac": 95, + "test_mtir_integration.jac": 4, + "test_visit_routing.jac": 13 + }; + +test "stdout scraping does not grow" { + measured: dict[str, int] = {}; + for name in test_files() { + n = count(r"in stdout_value|getvalue\(\)", read_test(name)); + if n > 0 { + measured[name] = n; + } + } + ratchet( + "stdout scraping", + measured, + STDOUT_GREP_BASELINE, + "ceiling", + "Assert on the value the function returned and on llm.sent(\"messages\") / " + + "llm.sent(\"tools\"). A stdout match cannot tell a wrong answer from a " + + "missing print, and it cannot see the request at all." + ); +} + + +# --------------------------------------------------------------------------- +# 5. Floors. These may only rise. +# Assert count is here because a deleted fixture can take its checks with it +# without changing the test count: 66 of the suite's asserts live in fixtures, +# guarded by 10 sentinel greps. +# --------------------------------------------------------------------------- +glob ASSERT_FLOOR_TESTS: int = 776, + ASSERT_FLOOR_FIXTURES: int = 66, + ANNEX_IMPORT_FLOOR: int = 2; + +test "total assert count does not fall" { + in_tests = 0; + for name in test_files() { + in_tests += count(r"(?m)^\s*assert\s", read_test(name)); + } + in_fixtures = 0; + for rel in fixture_files() { + in_fixtures += count(r"(?m)^\s*assert\s", read_fixture(rel)); + } + total = in_tests + in_fixtures; + floor = ASSERT_FLOOR_TESTS + ASSERT_FLOOR_FIXTURES; + assert total >= floor , ( + f"assert statements fell to {total} ({in_tests} in test files, " + + f"{in_fixtures} in fixtures); the floor is {floor}.\n\n" + + "Deleting a fixture deletes its asserts, and the test count does not " + + "move, so this is the only check that notices. Move each assert into the " + + "test that owned the fixture before deleting it.\n\n" + + "If a check is genuinely redundant, lower the floor here and name the " + + "dropped assert in the PR description." + ); +} + +test "annex adoption does not fall" { + n = 0; + for name in test_files() { + if name != ANNEX and "import from support_tests" in read_test(name) { + n += 1; + } + } + assert n >= ANNEX_IMPORT_FLOOR , ( + f"{n} test files import support_tests, the floor is " + + f"{ANNEX_IMPORT_FLOOR}.\n\n" + + "The annex is the one place a fake, an MTRuntime or a response shape is " + + "defined. A file that stops importing it has grown its own copy." + ); +}