jsteer/docs/reviews/code_demo.md at main

mirror of https://github.com/wassname/jsteer.git synced 2026-09-09 11:25:03 +08:00

Files

T

wassnameandClaudypoo 69d7d2f1d9 demo: apply gpt-5.5 review -- float-C fix, trim comments, soften overclaims

External review (docs/reviews/code_demo.md) triaged scout-mindset:
- FIX float-C crash: C={C:+g} not {:+d} (steering coeffs are floats)
- ACCEPT trim: shorter demo.py/config/fit docstrings (user also flagged verbosity)
- ACCEPT soften "fit J where we steer" -> "closer to the chat distribution" (most
  fitted positions are user/doc tokens, not assistant <think>; run-524 went further)
- ACCEPT soften "what the model thinks" -> "lens readout (linear approx)"
- ADD seed to show_steer so per-C blocks are comparable under sampling
- REJECT "</think> stripped by skip_special_tokens" -- verified false: decode keeps
  think tags (they're added, not registered-special tokens), split_think works

Co-Authored-By: Claudypoo <288921227+claudypoo@users.noreply.github.com>

2026-07-10 15:02:42 +08:00

2.7 KiB

Raw Permalink Blame History

Code Review: demo/chat-template steering updates

Summary

Adds shared demo rendering, chat-templated fitting, and refreshed notebooks/README. The pullback/VJP terminology is basically correct, but several comments/docs overclaim what the demo proves, and there are a couple of concrete runtime/API bugs.

Important (should fix)

jsteer/demo.py:80 tok.decode(..., skip_special_tokens=True) likely removes Qwen3’s </think> token before split_think() runs, so the demo will not actually separate the <think> trace from the answer. Split on generated token IDs first, or decode with skip_special_tokens=False for parsing and strip special tokens afterward.
config.py:19-31, scripts/fit.py:1-8, README.md:33-35 overclaim “we fit J where we steer.” jlens averages over token positions; wrapping WikiText as a chat user message still means most fitted positions are user/document tokens, not assistant <think> generation tokens. This is a reasonable distributional move, but not an exact chat/thinking operating-point fit. Reword to “closer to the chat prompt distribution,” or fit on assistant/thinking continuations / position-select the assistant tokens if that exact claim is needed.
jsteer/demo.py:84 formats C with +d, so show_steer(..., Cs=(-2.5, 0, 2.5)) crashes. Steering coefficients are naturally floats. Use C={C:+g} or similar.

Suggestions

jsteer/demo.py:1-21, config.py:1-8, scripts/fit.py:1-9, nbs/word_steering.ipynb, nbs/persona_steering.ipynb: comments/docstrings are much too explanatory for demo code. Trim the “why this exists,” “SHOULD/ELSE,” “Tufte,” and repeated Claude-authored rationale. Keep short usage notes and move caveats to README/docs if needed.
jsteer/demo.py:13-20, jsteer/jacobian.py:268, notebooks: “literally what the model is thinking” is too strong. lens_topk is an approximate Jacobian-lens readout of the current activation under active hooks, using a fitted linear map. Under large steering it can be off-linearization. Reword to “lens readout” / “decoded linear readout,” not literal thought.
jsteer/demo.py:56-58: using the model’s generation_config by default is defensible for Qwen demos, but the comments overgeneralize “greedy loops” as if universal. Also, stochastic sampling makes per-C comparisons noisy. Consider optional generation_kwargs and/or a seed for demo reproducibility.

Positive

README.md and nbs/persona_steering.ipynb clearly mark persona methods as experimental and mention the failed specificity controls.

Verdict

REQUEST CHANGES
Fix the <think> parsing bug and float-C crash, then trim/soften the overclaimed demo explanations before merging.

2.7 KiB Raw Permalink Blame History Unescape Escape