Skip to content

thinking-probes: read the reasoning field under both names#13

Open
Defilan wants to merge 1 commit into
TheTom:mainfrom
Defilan:fix-runner-server-compat
Open

thinking-probes: read the reasoning field under both names#13
Defilan wants to merge 1 commit into
TheTom:mainfrom
Defilan:fix-runner-server-compat

Conversation

@Defilan

@Defilan Defilan commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #8. @Blackwellboy's fallback landed in spine-probes/run_probes.py, but the two thinking probes still read reasoning_content only.

That is the worse place to be missing it, because those two tools exist specifically to measure whether a model reasons:

  • thinking_ab.py would classify every arm as NO_REASONING on a vLLM lane, reporting that the model never thinks
  • persona_ab.py would show 0 reasoning in all four cells, reporting the persona gate as perfectly effective including in the control

Both are fabricated results, and both are the same class of error as the enable_thinking claim these tools were written to correct: concluding from a field instead of measuring the thing.

Verified the parse path both ways:

vLLM (reasoning)                 -> REASONED_PARSED
llama.cpp (reasoning_content)    -> REASONED_PARSED
neither field                    -> NO_REASONING

README gains the field-name note plus one guard rail: NO_REASONING across every arm including true means check the field name before believing it, since a stack exposing thinking under a third name would still read as silent.

All three scripts still run and exit clean. No em or en dashes in the changed files.

Assisted-by: Claude Code (Opus), reviewed by me

@Blackwellboy's offlabel#8 fix landed in spine-probes/run_probes.py but not in
the thinking probes, which is the worse place to be missing it: these two tools
exist specifically to measure whether a model reasons.

llama.cpp populates reasoning_content; vLLM exposes reasoning. Reading only the
first makes every arm on a vLLM lane classify as NO_REASONING, so thinking_ab
would report a model as never thinking and persona_ab would report the persona
gate as perfectly effective in all four cells. Both are fabricated results, and
they are the same class of error as the enable_thinking claim these tools were
written to correct: concluding from a field rather than measuring the thing.

README gains the field-name note and one guard rail: NO_REASONING across every
arm including true means check the field name before believing it, since a
stack exposing thinking under a third name still reads as silent here.

Signed-off-by: Christopher Maher <chris@mahercode.io>
@Blackwellboy

Copy link
Copy Markdown
Contributor

Good catch on the two probes, and you are right that they were the worse place to be missing it. A tool whose whole job is to detect reasoning reporting NO_REASONING on every arm is a fabricated result that looks like a finding, which is the same failure mode twice over.

One mechanical thing, not about the change itself: the diff carries two compiled artifacts.

scripts/thinking-probes/__pycache__/persona_ab.cpython-314.pyc
scripts/thinking-probes/__pycache__/thinking_ab.cpython-314.pyc

I did exactly this on #9 and caught it just before it merged. The repo root .gitignore currently only holds the confidentiality entries, so there is nothing stopping __pycache__ from landing. git rm -r --cached scripts/thinking-probes/__pycache__ clears it, and a __pycache__/ line in .gitignore would stop it happening to any of us again.

The guard rail in the README is the part I like most here. NO_REASONING across every arm including true is exactly the signature that should make someone check the field name rather than believe the number, and a stack exposing thinking under a third name would still read as silent.

@Blackwellboy

Copy link
Copy Markdown
Contributor

Reviewed the diff properly rather than just agreeing with the premise. Three things checked, two clean, one suggestion.

The fallback matches what merged into run_probes.py exactly, same order and same shape:

reasoning = (ch["message"].get("reasoning_content")
             or ch["message"].get("reasoning") or "")

reasoning_content first, reasoning second, empty string last. So a stack that populates both cannot change verdicts depending on which tool you ran, which is the property you want across a probe suite.

The parse paths you describe are the ones the code implements. verdict() is the three-branch function your table documents, and the fallback feeds its first branch, so both field names now reach REASONED_PARSED and the <think> marker path is untouched as the REASONED_UNPARSED fallback. Nothing else changed: README plus eight lines across the two probes, no behavioural edits smuggled in alongside.

On the guard rail, one suggestion. The README note is the right instinct and I would go one step further, because this failure mode is silent by construction and a documentation guard rail only works on someone who reads the documentation at the moment they need it. The condition you describe is machine-checkable: if every arm including true comes back NO_REASONING, that is either a genuinely non-reasoning model or a field-name miss, and the tool knows which arms it ran. A single line at summary time saying so would make it self-catching:

all arms NO_REASONING including true: check the field name before believing this

That turns your guard rail from advice into a positive control, which matters more here than usual, because the thing being measured is absence, and absence is exactly what a wrong key also produces.

On the third-field-name case specifically, your wording covers it correctly. Worth noting the partial protection people may not realise they have: a third-name stack that still leaves <think> in content gets caught by REASONED_UNPARSED. The genuinely silent case is a third field name and clean parsing, which is the one your note is for.

I am updating trap 1 in our own template-traps registry to list these two probes as affected surfaces alongside the spine runner. That entry previously read as three occurrences of the field-name problem, and with these it is five, across three different tools. Worth recording because the pattern is now clearly "any tool that reads a reasoning field" rather than "a bug that happened to some scripts".

Separate mechanical note in my earlier comment about the two .pyc files in the diff.

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.

2 participants