Repository navigation
Closes #576: Name agent and max_output_tokens in verbosity remedy - #783
JhaSourav07 wants to merge 7 commits into
Conversation
…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>
[Low impact] The PR appears safe to merge; no new actionable issues were found.
|
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].
|
Addressed both review findings in the latest commits:
|
…data in optimize command
|
Thanks @JhaSourav07. The remedy line is genuinely better than what it replaces, and you fixed the dead 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 Blocking1. Applying the remedy makes the reported recoverable figure go UP.
2.
3. Parses fine ( 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:
The current middle position states a cap that does not cap and changes the numbers as a side effect. 5. The 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
What passedRound-trip is clean: What I ranFull non-e2e suite ( Start with 4, since the answer to it decides how much of 1 and 2 survives. |
|
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. I also checked the figure problem with the formula as written: 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 What still needs doing:
Credit where due: your card no longer advertises One process note: CI has never run on this PR. The fork-approval gate has it at |
|
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. |
…emove max_output_tokens config field
|
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 |
Summary
This PR updates
tj optimize verbosityand the Web UI to name the concrete agent and the exact parameters (max_tokensin application code, ormax_output_tokensin[agents.<agent_id>]) in the remedy line, replacing the old generic advice.max_output_tokenstoAgentConfigintokenjam/core/config.pywith TOML parsing and serialization roundtrip.AgentConfig.max_output_tokensinoutput_verbosity.pyso the verbosity analyzer reads and respects the configured agent cap when evaluating output baselines._render_verbosityintokenjam/cli/cmd_optimize.pyto point directly to cappingmax_tokensin the agent's application code (or via config) and measuring withtj optimize verbosity(fixing the broken--validatereference).tokenjam/ui/index.htmlto format complete per-agent table references and documentation indocs/optimize/verbosity.md.Related issue
Closes #576
Checklist
pytest tests/unit/ tests/synthetic/ tests/agents/ tests/integration/)ruff check tokenjam/)mypy tokenjam/)tests/factories.py(not rawNormalizedSpan)@anilmurtyas reviewer (or @-mentioned him above)Co-Authored-By: anshss noreply@github.com