thinking-probes: read the reasoning field under both names#13
Conversation
@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>
|
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. I did exactly this on #9 and caught it just before it merged. The repo root The guard rail in the README is the part I like most here. NO_REASONING across every arm including |
|
Reviewed the diff properly rather than just agreeing with the premise. Three things checked, two clean, one suggestion. The fallback matches what merged into reasoning = (ch["message"].get("reasoning_content")
or ch["message"].get("reasoning") or "")
The parse paths you describe are the ones the code implements. 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
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 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 |
…elf-checking Two review items from @Blackwellboy on TheTom#13. Mechanical: the branch carried two .pyc files under scripts/thinking-probes/__pycache__. Untracked them, and added __pycache__/ and *.pyc to .gitignore, which previously held only the confidentiality entries and so had nothing stopping this. Verified by regenerating the cache with py_compile and confirming git now ignores it. Substantive: the README told the reader to check the field name when every arm comes back NO_REASONING, but a guard rail only works on someone reading it at the moment they need it. The condition is machine-checkable, so both sampling tools now print it at summary time. That turns advice into a positive control, which matters here because the thing being measured is absence and a wrong key produces absence too. The check stays quiet whenever it would be guessing: - nothing scored at all (every request errored): no conclusion available - the thinking-enabled arm errored rather than returning nothing: the arm that would carry the signal was never measured - any REASONED_PARSED or REASONED_UNPARSED anywhere: the reader works The two tools key on different things and are not copies of each other. thinking_ab.py derives the enabled arm from ARMS rather than hardcoding "true", so it stays correct if the arms change. persona_ab.py has no true arm at all: its false cells are the structural control where zero is the expected result, so it keys on the absent cells instead. Signed-off-by: Christopher Maher <chris@mahercode.io>
|
Both fixed in 923119f, and thanks for reviewing the diff rather than the premise. Checking that the fallback matches The The guard rail. Took it, and you were right that documentation is the weaker form. Both tools now print at summary time: Two things I did deliberately that are worth flagging, because a positive control that fires wrongly is worse than none. It stays quiet whenever it would be guessing. Specifically when the thinking-enabled arm errored rather than returned nothing: that is the case where the arm carrying the signal was never measured, and blaming a field name would be inventing a diagnosis. Same when every request failed. I wrote ten cases covering both directions and confirmed the guard fires on the real signature and stays silent on each of the others, including against the committed blob rather than my working copy. The two tools are not copies. On your registry entry: five occurrences across three tools sounds right, and "any tool that reads a reasoning field" is the correct generalization. I would add that the class is narrower than it looks in one useful way. A third-name stack that still leaves Separately, your #16 lands on something I need to check on my own stack rather than assume transfers, so I have replied there. |
|
Nice work, and the part I'd highlight for anyone reading later is the guard staying silent when the enabled arm errored. A positive control that fires on a case it cannot diagnose is worse than no control, and ten cases run against the committed blob rather than the working copy is the right bar. The ARMS derivation versus the absent-cell keying is the sort of asymmetry that gets flattened by a well-meaning refactor, so having it in both docstrings is worth more than it looks. Taking your narrowing on the registry entry: a third field name plus clean server-side parsing is the genuinely silent case, and REASONED_UNPARSED catches the rest. That is more useful to someone diagnosing than my broader version, so the entry is being reworded to say it that way. The registry is moving to its own repo shortly since it is not Laguna specific, and contributions are open when it lands. Looking forward to your #16 result on your stack. That one needs a second stack more than most, since the whole mechanism is template-rendering behavior and yours renders differently. |
2 participants
Follow-up to #8. @Blackwellboy's fallback landed in
spine-probes/run_probes.py, but the two thinking probes still readreasoning_contentonly.That is the worse place to be missing it, because those two tools exist specifically to measure whether a model reasons:
thinking_ab.pywould classify every arm asNO_REASONINGon a vLLM lane, reporting that the model never thinkspersona_ab.pywould show 0 reasoning in all four cells, reporting the persona gate as perfectly effective including in the controlBoth are fabricated results, and both are the same class of error as the
enable_thinkingclaim these tools were written to correct: concluding from a field instead of measuring the thing.Verified the parse path both ways:
README gains the field-name note plus one guard rail:
NO_REASONINGacross every arm includingtruemeans 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