Skip to content

--deep concept pass: a stringified nodes tool argument is dropped by clean(), and the empty batch prints a ✓ #383

Description

@SeatownSin

Summary

ChatSynthesizer.synthesize() forces a tool call to record_graph, then passes
call.args.nodes straight into clean(), which opens with:

function clean(nodes) {
    if (!Array.isArray(nodes))
        return [];

Under a forced tool_choice, models routinely double-encode an array argument, so
nodes arrives as a JSON string rather than an array:

{"nodes": "[{\"name\": \"...\", \"type\": \"system\", \"summary\": \"...\"}]"}

The guard discards the entire payload and the run prints:

✓ concepts: 0 nodes, 0 links from 15 files

— with a ✓, and nothing else. Two twenty-minute runs produced that identical line before a
raw-traffic capture showed the model had in fact returned a complete, correct 15-node graph
each time.

This is not covered by the existing guards, and I want to be precise about why, because
the adjacent fixes all look like they should catch it:

Still present in 0.18.0 (dist/ai/synthesize.js, clean() at line 60,
return clean(nodesFromResponse(res)?.nodes) at line 111).

Environment

Evidence

Captured through a logging proxy sitting between graft and the endpoint:

finish_reason : tool_calls
completion    : 3,148 tokens (513 reasoning)
tool_calls[0] : record_graph
arguments     : {"nodes": "<12,012-char string>"}

That string contains 15 well-formed nodes — 10 system, 5 concept, 28 links, every one
with a summary — and they are accurate; I spot-checked them against the source. It is also
slightly malformed (a stray } at char 1204), so a bare JSON.parse of it fails too, which
matters for the fix.

Worth noting this is model-dependent, not universal: on a different, much larger repository the
same graft build produced 704 concept nodes across 2,783 files with zero concept-pass errors,
because that model serialized the argument correctly. A user whose model double-encodes gets an
empty concept layer and a ✓.

Suggested fix

Two parts, and I think the second is the more valuable one.

1. Coerce before the guard. If nodes is a string, JSON.parse it; on failure, salvage
balanced {"name": ...} objects from it. That recovers all 15 nodes on this fixture, including
past the stray brace.

2. Never return an empty batch in silence. A 0-node batch should report the reply size, the
tool-call count, and the shape actually received. A reply carrying a full architecture graph and
a reply carrying nothing must not print the same line — and the current line prints a ✓ for
both. This generalises past this particular bug: whatever the next provider serialization quirk
turns out to be, one run should be enough to see it rather than two.

clean() returning [] for a non-array is reasonable as a last-ditch guard; the problem is
that nothing above it distinguishes "wrong type" from "genuinely empty", and nothing prints the
difference.

Verified locally on the same fixture and model, meanings replayed from cache:

⚠ synthesize: tool call returned "nodes" as a 12012-char string, not an array
  — salvaged 15 node(s) from malformed JSON
✓ concepts: 15 nodes, 27 links from 15 files (0 read, 15 cached)

About 70 lines against dist/ai/synthesize.js. Happy to open a PR against source if that is
easier than a patch against dist/.

A note on what already works well

The resumable cache is what made this diagnosable at all — every re-run replayed the meaning
tier instantly, so iterating on the concept pass cost nothing. Worth saying, since the rest of
this report is a complaint.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions