Skip to content

Closes #576: Name agent and max_output_tokens in verbosity remedy - #783

Open
JhaSourav07 wants to merge 7 commits into
Metabuilder-Labs:mainfrom
JhaSourav07:fix/576-verbosity-remedy-agent-config
Open

JhaSourav07 wants to merge 7 commits into
Metabuilder-Labs:mainfrom
JhaSourav07:fix/576-verbosity-remedy-agent-config

Conversation

@JhaSourav07

@JhaSourav07 JhaSourav07 commented Oct 16, 2026 •

Copy link
Copy Markdown

Summary

This PR updates tj optimize verbosity and the Web UI to name the concrete agent and the exact parameters (max_tokens in application code, or max_output_tokens in [agents.<agent_id>]) in the remedy line, replacing the old generic advice.

  • Added max_output_tokens to AgentConfig in tokenjam/core/config.py with TOML parsing and serialization roundtrip.
  • Connected AgentConfig.max_output_tokens in output_verbosity.py so the verbosity analyzer reads and respects the configured agent cap when evaluating output baselines.
  • Updated _render_verbosity in tokenjam/cli/cmd_optimize.py to point directly to capping max_tokens in the agent's application code (or via config) and measuring with tj optimize verbosity (fixing the broken --validate reference).
  • Updated the inline remedy text in tokenjam/ui/index.html to format complete per-agent table references and documentation in docs/optimize/verbosity.md.
  • Added unit tests covering the new config key, analyzer config-reading, and output rendering.

Related issue

Closes #576

Checklist

  • Tests pass (pytest tests/unit/ tests/synthetic/ tests/agents/ tests/integration/)
  • Lint clean (ruff check tokenjam/)
  • Type check clean (mypy tokenjam/)
  • Test spans use tests/factories.py (not raw NormalizedSpan)
  • Requested @anilmurty as reviewer (or @-mentioned him above)

Co-Authored-By: anshss noreply@github.com

…ty remedy

The verbosity analyzer previously output generic remedy advice
("add a terse system-prompt line and/or a max_tokens cap") without
pointing the user to any specific agent, file, or configuration key.

Introduce the `max_output_tokens` field on `AgentConfig` under
`[agents.<agent_id>]` to provide a concrete configuration setting
for capping agent output tokens. Update the CLI and Web UI renderers
to explicitly identify the flagged agent(s) and name the configuration
key, matching the actionable format used by sibling analyzers.

Co-Authored-By: anshss <noreply@github.com>
@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Updates verbosity advice in the CLI, Web UI, and docs.

  • Verbosity remedies name the agent and the command to measure results.

The earlier config-related findings are addressed: the remedies no longer suggest TOML settings, and the analyzer uses the guarded cohort median rather than a configured per-call cap. All supplied previous threads are unnumbered, so they require no numbered classifications.

Reviews (6) · Last reviewed commit: "Merge branch 'main' into fix/576-verbosi..." · Reviewed by Greptile

Comment thread tokenjam/cli/cmd_optimize.py Resolved
Comment thread tokenjam/ui/index.html Resolved
Update verbosity remedy to point users to capping max_tokens in their
agent's application code (where LLM calls actually execute) and
measuring via `tj optimize verbosity` (fixing the broken
`tj optimize --validate` reference).

Additionally, connect `AgentConfig.max_output_tokens` in
`output_verbosity.py` so that when a configured limit is present in
`tokenjam.toml`, the analyzer reads it as the effective baseline.
Build a complete [agents.<id>] and agent application code reference
for each agent before joining them, preventing malformed headers like
[agents.reviewer, writer].
@JhaSourav07

Copy link
Copy Markdown
Author

Addressed both review findings in the latest commits:

  1. Application code limit & analyzer connection: Updated _render_verbosity to point users directly to capping max_tokens in the agent's application code (where model calls actually execute) and directed them to measure with tj optimize verbosity (fixing the broken tj optimize --validate reference). Connected AgentConfig.max_output_tokens in output_verbosity.py so the analyzer reads and respects the configured agent cap when evaluating output baselines.
  2. UI TOML formatting: Built complete individual agent references (agent <id> application code (or [agents.<id>] max_output_tokens)) before joining them, preventing malformed headers like [agents.reviewer, writer].

@anilmurty

Copy link
Copy Markdown
Contributor

Thanks @JhaSourav07. The remedy line is genuinely better than what it replaces, and you fixed the dead tj optimize --validate reference that #782 is about. I verified that: tj optimize verbosity resolves and exits 0, while --validate verbosity still errors, so the card no longer prints a command that cannot run.

Not ready to merge yet. The analyzer changes are the problem, and two of them move a dollar figure in the wrong direction.

Heads up on CI: it has never run on this PR. The fork-approval gate has it at action_required, so there is no green to read. I ran the suite locally instead (details at the bottom).

Blocking

1. Applying the remedy makes the reported recoverable figure go UP.

effective_baseline = min(median, cap_limit). A user reads the card, sets [agents.X] max_output_tokens = 500, re-runs, and every session in that cohort is now measured against 500 instead of its cohort median. More sessions clear the 2.0x gate, over_baseline_tokens grows, recoverable_usd grows. The line literally says "then measure with tj optimize verbosity", and that measurement rewards them for doing nothing and punishes them for complying. Greptile's P1 on line 326 is the same finding from the other side: the query sums all calls in a session, so a per-call cap gets compared against a session total, and a fully compliant session with three 150-token calls under a 150 cap is flagged with 300 recoverable tokens.

2. suggested_max_tokens silently changed basis.

baselines.append(...) moved inside the for m in members loop. It used to collect one entry per qualifying cohort; it now collects one per session, so the median is weighted by cohort size. Three cohorts of 5/5/20 sessions with medians 100/200/5000 give 200 before this PR and 5000 after. Nothing in the diff mentions it, no test covers it, and the comment four lines below still reads "the median cohort baseline", as does docs/optimize/verbosity.md. That is a 25x move in a number we print to users.

3. max_output_tokens = 0 takes down the whole verbosity pass.

Parses fine (int(0) is not None), then effective_baseline is 0, the <= gate does not catch it, and round(m.output_tokens / 0, 2) raises ZeroDivisionError. Negatives give negative multiples and recoverable counts above actual output. Also, a non-integer value raises a bare ValueError straight out of _parse. Needs a positive-integer check at load. Greptile P1 on line 350, confirmed.

4. The key still does not cap anything, which is the heart of #576.

Greptile's first P1 was right and I do not think the follow-up commits answered it. We never issue the model call, so no key in our config can cap an agent's output. Wiring it into the analyzer's own baseline instead made it a detection-tuning knob, which is a different thing and is what produces findings 1 and 2. Two honest ways out, and I would like your view before you pick:

  • Name only the real lever: the agent id plus max_tokens in their application code. Drop the config key.
  • Keep the key, but as a declared intent we read back for reporting only, and say so plainly in the string, with no effect on the baseline.

The current middle position states a cap that does not cap and changes the numbers as a side effect.

5. The _refresh_shipped_via_daemon isinstance guard is unrelated.

Different function, different code path, no test, not in the PR description. Pull it out and send it separately if it is fixing something real.

Nits

  • With several flagged agents the sentence breaks down: "cap max_tokens near ~500 in agent a3 (or [agents.a3] max_output_tokens) / agent a4 (...) / agent a5 (...) application code". Five agents is routine since MAX_EXAMPLES is 5. It also only names agents that survived that truncation, not every flagged agent.
  • docs/optimize/verbosity.md:25 still says --validate is "load-bearing, not optional" for verbosity. You touched this file, so worth fixing the same dead instruction here.
  • docs/configuration.md is the full TOML surface and does not list the new key.

What passed

Round-trip is clean: _serialise walks dataclass fields generically, None is omitted, and an existing config without the key parses unchanged. The honesty caveat constant and estimate_basis are untouched. The UI module still parses (tests/unit/test_lens_module_parses.py), no binding was removed, and Greptile's P2 on the invalid TOML header in the UI is addressed by fe89f5f.

What I ran

Full non-e2e suite (tests/unit tests/synthetic tests/integration tests/agents, -n auto) on this branch and on main: identical results, 5 failures that reproduce on main and are local environment artifacts, not yours. ruff check tokenjam/ clean, mypy tokenjam/ clean across 288 files. Rendered the verbosity card for one agent, five agents, and a null agent id. Invoked tj optimize verbosity, tj optimize --validate verbosity, and tj optimize --validate.

Start with 4, since the answer to it decides how much of 1 and 2 survives.

@anilmurty

Copy link
Copy Markdown
Contributor

Follow-up from me, and it shortens the rework rather than lengthening it.

I have amended #576: the config key comes out. The issue asked for something that cannot be built, and that is my error, not yours.

TokenJam does not issue the model call. max_tokens is a parameter on the request your agent's own code sends to the provider, so no key in our config can cap anything. A [agents.<id>] max_output_tokens key would advertise a control we do not have.

I also checked the figure problem with the formula as written:

no cap declared        baseline=600   flagged=2   over-baseline tokens=2100
cap declared at 300    baseline=300   flagged=2   over-baseline tokens=2700
cap declared at 200    baseline=200   flagged=4   over-baseline tokens=3600

Declaring a tighter cap reports more recoverable. The same line tells the user to re-run and measure, so the measurement rewards ignoring the advice. That is not a tuning problem, it is what happens whenever a user-declared number is folded into a baseline the same report prices against.

What to keep: naming the agent. That was the actual point of #576 and you got it right.

What to drop: the AgentConfig field, its parse and round-trip, and the effective_baseline = min(median, cap_limit) change. With that gone, the suggested_max_tokens basis change and the max_output_tokens = 0 crash both go with it, so three of the four blocking items disappear.

What still needs doing:

  • Point at the real lever: the max_tokens parameter in the agent's own code, and its system prompt. Name the agent, say where the lever lives.
  • docs/optimize/verbosity.md:25 still calls --validate "load-bearing, not optional" for verbosity. It errors for verbosity (Verbosity finding advertises tj optimize --validate, which rejects verbosity #782), and your PR touches that file.
  • Drop the _refresh_shipped_via_daemon isinstance change; unrelated to this issue and untested. Happy to take it as its own PR if you hit a real bug there.

Credit where due: your card no longer advertises tj optimize --validate, which fixes #782's reported symptom on this surface. I verified by invoking it rather than reading. #782 stays open for the real measured validate, but you closed the dead-command half.

One process note: CI has never run on this PR. The fork-approval gate has it at action_required, so the checks you see are not a pass. Anil will start the run; everything above is from local runs on my machine, which is not a substitute for the 3.10/3.11/3.12 matrix.

@anilmurty

Copy link
Copy Markdown
Contributor

CI has now run and is green across the matrix: lint, 3.10, 3.11, 3.12, test-ts, version-lockstep. That retires the "unproven" caveat at the top of my review — the findings themselves are unchanged, since none of them are things CI would catch. The figure behaviour and the inert config key are both correct-by-the-tests.

Comment thread tokenjam/cli/cmd_optimize.py Resolved
@JhaSourav07

Copy link
Copy Markdown
Author

Thanks @anilmurty for the thorough review and the clarification on #576. That makes total sense—since TokenJam does not issue the model call, advertising a config key to cap tokens would be promising a control we don't have, and folding it into the baseline caused the perverse incentive where tighter caps reported more waste.

I have updated the branch to address all the feedback

This branch has not been deployed

No deployments
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.

tj optimize verbosity remedy tells the user to fix something but names no file, line, or config key

2 participants