Skip to content

implement token reduction with headroom#12

Merged
sayakpaul merged 4 commits into
mainfrom
implement-headroom
Jun 8, 2026
Merged

implement token reduction with headroom#12
sayakpaul merged 4 commits into
mainfrom
implement-headroom

Conversation

@sayakpaul

Copy link
Copy Markdown
Member

Fixes #6

I am guessing we need to propagate the changes to the UI as well as that is the only way to use the bot at the moment?

@sergereview sergereview Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤗 Serge says:

The PR correctly wires headroom context compression into every LLM call path, so all three modes are functionally covered (GitHub Action → action_runner.py, GitHub App webhook → app.py, staged web app → webapp.py). The compressor is attached to the single ChatCompletionClient instantiation point in reviewer.py, which means run_review, run_followup, prepare_review, and the synthesis helper all compress before posting.

However, the Action interface is incomplete: action.yml only exposes 3 of the 7 documented headroom settings as inputs, and the run: block only forwards those 3. Action users who read the README table cannot actually set HEADROOM_COMPRESS_SYSTEM_MESSAGES, HEADROOM_PROTECT_RECENT, HEADROOM_MIN_TOKENS, HEADROOM_KOMPRESS_MODEL, or HEADROOM_MODEL_LIMIT via with:. They would have to fall back to workflow-level env: variables, which contradicts the README’s “Env var / input” column.

Action interface gaps

  • action.yml needs inputs (and run: forwarding) for the missing headroom settings so Action users can configure them the same way App/Web operators can via env vars.

Documentation gaps

  • .env.example does not mention any HEADROOM_* variables, making the feature undiscoverable for App/Web deployments.

Tests

  • tests/test_compression.py has good coverage of the fallback paths (missing package, disabled, empty messages, failure, model forwarding). No issues there.

Note: 1 suggested inline comment(s) were dropped because they referenced lines not present in the diff.

26 LLM turns · 36 tool calls · 1007.9s · 560483 in / 58026 out tokens

Comment thread action.yml
headroom_compress_user_messages:
description: 'Set to "true" to also compress user messages (the annotated diff). Off by default so cited line numbers stay intact.'
required: false
default: 'false'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The run: block only forwards three headroom env vars, but the README documents seven. Action users cannot configure HEADROOM_COMPRESS_SYSTEM_MESSAGES, HEADROOM_PROTECT_RECENT, HEADROOM_MIN_TOKENS, HEADROOM_KOMPRESS_MODEL, or HEADROOM_MODEL_LIMIT via with: inputs because there are no corresponding action inputs.

Please add the missing inputs and forward them in the run: step so the Action interface matches the README table.

Comment thread action.yml
HEADROOM_COMPRESS: ${{ inputs.headroom_compress }}
HEADROOM_TARGET_RATIO: ${{ inputs.headroom_target_ratio }}
HEADROOM_COMPRESS_USER_MESSAGES: ${{ inputs.headroom_compress_user_messages }}
run: reviewbot-action

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing forwarding for HEADROOM_COMPRESS_SYSTEM_MESSAGES, HEADROOM_PROTECT_RECENT, HEADROOM_MIN_TOKENS, HEADROOM_KOMPRESS_MODEL, and HEADROOM_MODEL_LIMIT.

@sergereview sergereview Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤗 Serge says:

This PR adds optional context compression via the headroom-ai package. The implementation is clean, well-tested, and follows safe fallback patterns: compression is opt-in, import failures and runtime errors are caught and silently fall back to uncompressed messages, and the Action only installs the heavy extra when the input is enabled. Documentation, env examples, and Action inputs are all consistent. No correctness or security issues found.

1 LLM turn · 0 tool calls · 607.2s · 11497 in / 8545 out tokens

@sayakpaul sayakpaul merged commit 7c5a121 into main Jun 8, 2026
1 check passed
@sayakpaul sayakpaul deleted the implement-headroom branch June 8, 2026 10:39
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.

Use token reduction tools

1 participant