fix(mcp): emit trace evidence columns in header order

[rudi193-cmd]

What does this PR do?

The original #1542 json omission landed in v0.10.3. Two leftovers remained on main:

1. bfs_to_tree_json declared strategy, confidence before args but emitted args first, so json + include_evidence + data_flow mislabeled columns (len(cols)==len(row) still held).
2. Tree flat_trace (risk_labels or data_flow) called bfs_to_toon_table without include_evidence, so evidence was a silent no-op on that branch.

This PR emits cells in header order and forwards the flag into the flat table. Tests assert column identity, not just count.

Not folding in #1597 budget signalling.

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects
  unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass locally (scripts/test.sh --suites mcp)
• Lint passes (make -f Makefile.cbm lint-ci)
• New behavior is covered by a test (reproduce-first for bug fixes)

Fixes #1542

Made with Cursor

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Thank you for the focused follow-up to #1542. This is the right review shape: one output-contract defect, two production branches, and tests that bind column identity rather than merely row width. The initial patch scan found no new dependency, execution, network, or configuration surface. It is labeled high priority because silently mislabeling trace evidence is a correctness bug. The review queue is full, but this contribution is now routed as a compact bug fix.

[DeusData]

Reviewed, and the change is right. Sorry it sat two weeks after the 18 August note.

First, your four red checks: none of them are your code, and the cause has already been fixed.

security / codeql-gate started at 03:38:15 and completed at 04:23:51 — 45 minutes and 36 seconds. That is the gate's old wait budget expiring, not a finding. It then cascaded: lint / lint-mem shows The operation was canceled with zero warnings of its own, and analyze and ci-ok fall out of the same run.

The gate's wait was raised from 45 to 150 minutes in #1960, merged earlier today. So a re-run should clear all four without you changing anything.

One thing to wait for before re-running: main does not currently compile — two changes landed the same type short-name index into type_registry.h and git merged them into duplicate struct members. #1993 is the repair and is nearly green. Re-run after that lands, or you will swap one unexplained red for another. I will trigger it from this side; there is nothing for you to do.

The review

Both defects are real and the fixes are the right shape.

Column order. Emitting cells in header order rather than declaration order is the fix, and the reason this survived is worth naming: len(cols) == len(row) still held, so a width assertion passed while every column past the divergence was mislabeled. A test that counts columns cannot see this class of bug at all. Yours assert column identity, which is why they would have caught it.

The dropped flag. flat_trace calling bfs_to_toon_table without include_evidence made the flag a silent no-op on that branch — the worst kind, because the caller has every reason to believe it worked.

And you caught the consequence of your own fix. const char *cols[5] was exactly right on main: qn, hop, risk, test, args. Adding strategy and confidence makes the maximum seven, and you resized to cols[7] rather than leaving a two-element overflow behind the new branch. That is the part of this change that would have been genuinely dangerous to miss, and it is easy to miss because the array is declared forty lines above the code that grows it.

The -/- placeholders for the root hop and non-CALLS edges keeping the column count fixed, and matching bfs_to_tree_table's existing convention rather than inventing a second one, is right too.

Holding scope out of #1597's budget signalling was the correct call.

Status: approved and cleared. It merges once main is green and the checks re-run.

[DeusData]

Merged as 5fbab7bb.

Two things about this one are worth recording rather than just closing.

The bug survived because the test that should have caught it was counting. len(cols) == len(row) held while every column past the divergence was mislabeled, so a width assertion passed on wrong data. Your tests assert column identity, which is the only shape that can see this class of defect. That distinction is worth more than the fix.

And you caught the consequence of your own change. const char *cols[5] was exactly right on main — qn, hop, risk, test, args. Adding strategy and confidence makes the maximum seven, and you resized to cols[7] rather than leaving a two-element overflow sitting behind the new branch. The array is declared about forty lines above the code that grows it, which is precisely the distance at which people miss this.

On the CI you were told about: your four red checks were never yours. security / codeql-gate ran 45 minutes and 36 seconds — the gate's old wait budget expiring, not a finding — and lint / lint-mem recorded "The operation was canceled" with zero warnings of its own, so the gating memory lane was never actually objecting. #1960 raised that budget to 150 minutes, and the re-run after main was repaired came back clean across the full matrix.

Sorry it sat two weeks after the 18 August note. Thank you for keeping it to one claim and holding #1597's budget signalling out.