Skip to content

test(byllm): a ratchet and a written contract for the test suite (#9002 phase 0) - #9004

Closed
MalithaPrabhashana wants to merge 1 commit into
jaseci-labs:mainfrom
MalithaPrabhashana:test/byllm-suite-gate
Closed

test(byllm): a ratchet and a written contract for the test suite (#9002 phase 0)#9004
MalithaPrabhashana wants to merge 1 commit into
jaseci-labs:mainfrom
MalithaPrabhashana:test/byllm-suite-gate

Conversation

@MalithaPrabhashana

@MalithaPrabhashana MalithaPrabhashana commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Phase 0 of #9002. No existing test or fixture changes; this only adds a ratchet and the
contract it enforces.

Why this first

Correction to an earlier draft. I first cited #8479 (+1,046 lines, merged 2026-09-04)
as inflow that ignored the new annex. It was opened on 2026-08-22, twelve days before the
annex existed, so its author could not have used it. The example was unfair and I have
withdrawn it. The evidence that matters is the live queue.

Nine open PRs touch this directory right now:

PR opened +lines to tests new fixtures MockLLM( SimpleNamespace( stdout grep uses annex
#8922 2026-09-03 890 2 1 7 0 no
#8809 2026-08-31 635 0 1 4 1 no
#8926 2026-09-03 479 1 0 1 0 no
#8953 2026-09-04 240 0 0 0 0 yes
#8826 2026-09-01 214 0 0 3 0 no
#7692 2026-07-23 87 0 0 0 1 no
#8955 2026-09-04 60 0 1 0 0 yes
#7568 2026-07-18 4 1 0 0 0 no
#8080 2026-08-12 2 0 0 0 0 no

About 2,600 lines, 4 new fixtures, 15 SimpleNamespace fakes and 3 MockLLM
constructions are queued to land, and only two of the nine use the annex. Most predate
it; nobody is ignoring a rule. But nothing tells an author at rebase time that the rule
changed, so they merge as written and #8930's consolidation restarts from behind. This
guard tells them, with the file, the delta and the fix. It is 499 lines that change no
behaviour, and every later PR in #9002 depends on it holding.

Scope note. This PR is the gate only. The shape it enforces is not invented here: it
is the one jaclang/scale/tests/server already uses (322 tests, zero fixtures, zero
stdout scraping), and the consolidation it protects is the one #8599 already did for the
main test corpus (+5,302/-8,992, 115 files, "no assertion was removed"). #8599 touched
zero byLLM files; byLLM is the suite it missed.

test_guard.jac

A ratchet, not a wall. Every number is today's measured count, not a target.

Ceilings, on the patterns being retired. These may only fall:

check today
fixture programs 66
fixtures that assert or print 52
fixtures constructing a MockLLM 53
MockLLM( in test files per file: compaction 14, usage 5
SimpleNamespace( in test files per file: usage 19, byllm 10, telemetry 8, tool_arg_streaming 7, compaction 4
stdout scraping per file: byllm 95, visit_routing 13, mtir 4, annex 2

Floors, on what is being built. These may only rise:

check today
assert statements 776 in test files + 66 in fixtures
test files importing support_tests 2

Lowering a ceiling or raising a floor while cleaning up is a one-line edit in the same
PR and needs no justification; that is the intended direction. Moving one the wrong way
fails with the file, the delta and the fix:

AssertionError: MockLLM constructions rose:
  test_visit_routing.jac: 1, not in the baseline at all

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.

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.

The assert floor is the one that is not obvious

66 of the suite's 842 assertions live inside fixture programs, guarded by only 10
sentinel greps:

fixture asserts inside
logging_stream_react 13
stream_retry 12
conversation_param 12
logging_stream_no_tools 9
streaming_conversation 6
seven more 14

Delete conversation_param.jac, write one test that checks the return value, and the
test count does not move while 12 checks disappear. A deleted-test to surviving-row
mapping cannot see that. A total assert count can, which is why it is a gate rather
than a convention.

TESTING.md

The contract the guard enforces, in one page: fake the model at model_call_* so
make_model_params and parse_response are real code and the test can assert on the
request; a fixture is a program under test with no model, entry block, prints or
asserts; assert on values, never on stdout.

The fixture section documents five things I verified against the compiler before
writing any of this, each of which costs an afternoon if you meet it in review instead:

  1. by llm() resolves its model global at call time, so a test can own the fake and the
    fixture stays pure. The fixture needs glob llm: any = None;; the bare form infers
    NoneType and the test's assignment fails jac check with E1001.
  2. Graph fixtures need a static import of their archetypes. load_fixture() returns
    the module as any, so root ++> g.Desk() fails with
    E1097: Connection right operand must be a node instance, and CI runs
    JAC_TEST_STRICT=1. This invalidated my own first draft of the conversion recipe.
  3. Routing replies are generated handles, not indices. The return type is
    list[enum[RouteChoice]] and members come 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.
  4. Jac.jac_import returns a cached module and does not re-execute it. State carries
    over between calls. This contradicts the comment above test_byllm.jac:1618, which
    says the module is imported once per test so mock outputs are consumed exactly once.
    Those two tests pass only because they use disjoint llm objects.
  5. root accumulates across tests in a file. Per-test subgraphs stay isolated as long as
    you spawn on the node you just created rather than walking from root.

Validation

Each rule was checked by introducing the violation, confirming red, and restoring:

probe result
baseline 8 passed
add fixtures/zz_probe.jac red
add a MockLLM( to a test file red
add a SimpleNamespace( fake red
add a stdout grep red
delete one assert from a fixture red
drop an import from support_tests red
restored 8 passed

Suite, this branch, JAC_TEST_JOBS=2 JAC_TEST_STRICT=1:

before   199 passed, 1 skipped
after    207 passed, 1 skipped

jac fmt --check --lintfix and jac check clean on both files.

One observation for the record

On a single run, taken while the compiler itself was recompiling after a branch switch,
two pre-existing tests errored inside JacProgram().compile():

RuntimeError: scheduling error: pass OwnershipCheckPass requires analysis 'CfgSucc'
which is neither provided by an earlier pass in the schedule ... nor already available

async by llm() codegen emits await Jac.acall_llm not call_llm and
async by llm() end-to-end with mocked acall_llm, both in test_byllm.jac. It has not
reproduced since: seven subsequent runs are clean (once serial, three times parallel on
the full suite, four times on test_byllm.jac plus the guard), and a trivial eight-test
file in place of the guard does not trigger it. Neither file here compiles anything, so
this looks like a compiler pass-schedule race that surfaces when those two tests build
their own JacProgram while the compiler is being rebuilt, rather than anything this PR
introduces. Flagging it rather than leaving it in a log; happy to open a separate issue
if it recurs.

Ledger

before after
test blocks 242 250
table rows / cases unchanged unchanged
assert statements (test files) 776 782
assert statements (fixtures) 66 66
tests skipped 1 1
tests newly red, with issue number 0 0

No test deleted, no assert dropped. The eight added blocks and six added asserts are the
guard itself.

Next

PR 1 in #9002 is the routing cluster, the hard case: five fixtures sharing 47% of their
lines verbatim become one pure graph program, using the mechanics verified above. It
will be the first PR to lower a ceiling in test_guard.jac rather than only pass it.

…eci-labs#9002 phase 0)

The byLLM suite is 10,361 lines holding 242 test blocks, and it grows by
roughly one fixture per fix PR. The two consolidation PRs merged on 2026-09-04
(jaseci-labs#8936, jaseci-labs#8938) removed 280 lines; jaseci-labs#8479 added 1,046 back the same day on the
old seams, deleting the file jaseci-labs#8936 had just migrated. Net for the effort so
far is +766 lines. Consolidation alone does not hold.

This adds the piece that does.

test_guard.jac is a ratchet, not a wall. Every number in it is today's measured
count: ceilings on the things being retired (fixture programs, MockLLM
constructions, SimpleNamespace fakes, stdout scraping) and floors on the things
being built (assert count, annex adoption). Cleaning up means editing a number
down in the same PR, which is the intended direction and needs no explanation.
Adding one of the retired patterns fails here, with the file, the delta, and
what to use instead.

The assert floor is the one that is not obvious. 66 of the suite's 842
assertions live inside fixture programs, guarded by only 10 sentinel greps, so
deleting a fixture can drop a dozen checks while the test count does not move.
A deleted-test to surviving-row mapping cannot see that; a total assert count
can.

TESTING.md is the contract the guard enforces: fake the model at model_call_*
so make_model_params and parse_response are real code and the test can assert
on the request; a fixture is a program under test with no model, entry block,
prints or asserts; assert on values, never on stdout.

The fixture section documents five things verified against the compiler
beforehand, each of which costs an afternoon if missed: `by llm()` resolves its
model global at call time, so a test can own the fake, but the fixture needs
`glob llm: any = None;` or the assignment fails E1001; graph fixtures need a
static import of their archetypes, because `root ++> g.Desk()` on a module from
jac_import fails E1097; routing replies are generated handles, not indices;
`Jac.jac_import` returns a cached module rather than re-executing it, which
contradicts the comment above test_byllm.jac:1618; and per-test subgraphs stay
isolated only if you spawn on your own node rather than from root.

No existing test or fixture changes. The suite goes from 199 passed, 1 skipped
to 207 passed, 1 skipped.
@MalithaPrabhashana

Copy link
Copy Markdown
Collaborator Author

One detail a reviewer will probably catch, so stating it up front: the ledger says asserts in test files go 776 to 782, but the floor in test_guard.jac is 776, not 782.

That is deliberate. test_files() excludes test_guard.jac from its own count, so the guard's six asserts cannot mask assert deletions elsewhere in the suite. If the floor counted the guard, adding six asserts here would buy headroom to quietly drop six somewhere else. The floor tracks the suite it is guarding, not itself.

@MalithaPrabhashana

Copy link
Copy Markdown
Collaborator Author

Closing by decision. We will manage regrowth through review rather than a ratchet, and TESTING.md moves to the final conversion PR, where it can document the finished shape instead of the intended one.

Nothing here is lost. The verified mechanics from its fixture section (call-time resolution of the model global, the glob llm: any = None; requirement, static import for graph fixtures, generated routing handles, cached jac_import) are recorded in #9002 section 4 and will land with the doc. The branch stays for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant