Skip to content

feat(archive): full-file path in pointer + env-configurable knock-in/knock-out#68

Open
niemst wants to merge 1 commit into
alexgreensh:mainfrom
niemst:feat/archive-path-and-configurable-hysteresis
Open

feat(archive): full-file path in pointer + env-configurable knock-in/knock-out#68
niemst wants to merge 1 commit into
alexgreensh:mainfrom
niemst:feat/archive-path-and-configurable-hysteresis

Conversation

@niemst

@niemst niemst commented Jun 16, 2026

Copy link
Copy Markdown

What

  • Archived-result message now includes the on-disk file path (plus the expand id), so a full result is directly retrievable — not only via expand <key>.
  • Knock-in / knock-out thresholds are now env-configurable:
    • TOKEN_OPTIMIZER_ARCHIVE_THRESHOLD — knock-in X (min size to archive)
    • TOKEN_OPTIMIZER_ARCHIVE_PREVIEW_SIZE — knock-out Y (inline preview cap)
  • Invariant X > Y enforced: if misconfigured (Y ≥ X), Y is clamped just below X rather than crashing the hot-path hook.
  • Loaded the literal 1500 preview size into the named constant; build_archive_pointer gains an optional, backward-compatible archive_path arg.

Notes

  • Defaults unchanged (4096 / 1500) — behavior is identical unless the env vars are set.
  • Both mirrored copies (skills/… and plugins/token-optimizer/skills/…) updated identically.
  • py_compile clean; verified default / override / clamp / invalid-fallback at import time.

…knock-out

- archive replacement message now includes the on-disk file path (and expand id),
  so the full result is directly retrievable, not only via `expand <key>`
- _ARCHIVE_THRESHOLD (knock-in X) / _ARCHIVE_PREVIEW_SIZE (knock-out Y) are now
  env-tunable: TOKEN_OPTIMIZER_ARCHIVE_THRESHOLD / TOKEN_OPTIMIZER_ARCHIVE_PREVIEW_SIZE
- enforce X > Y: clamp Y below X on misconfig (never crashes the hot-path hook)
- replace the literal 1500 preview-size with the named constant
- build_archive_pointer gains an optional, backward-compatible archive_path arg

Defaults unchanged (4096/1500). Both mirrored copies updated.
@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Before we can merge this PR, you need to sign our Contributor License Agreement.

This project uses dual licensing (PolyForm Noncommercial + Commercial). The CLA ensures we can continue offering both licenses.

To sign, please post a comment with exactly:
I have read the CLA Document and I hereby sign the CLA


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@alexgreensh

Copy link
Copy Markdown
Owner

Thanks for digging into the archive path, @niemst — appreciate the careful defensive coding (the _positive_int_env clamp is genuinely tidy). After review I'm going to pass on this one as-is, and I want to be transparent about why since the two halves landed together:

Env-configurable thresholds — declining. The 4096/1500 constants aren't general config; they're the compression heuristic's core dial, validated against our harm/savings gate. Exposing them as knobs lets a setup silently drift net-negative with no gate re-validation, which cuts against Token Optimizer's "sensible defaults, no knob sprawl" design. There's also a correctness snag: TOKEN_OPTIMIZER_ARCHIVE_PREVIEW_SIZE only resizes the internal telemetry preview, not the model-visible replacement preview, so the knob doesn't do what its name promises.

File path in the archived message — good idea, but I'd land it differently. Surfacing a raw path as a fallback for harnesses where expand isn't wired is worth having. But as written it only updates the inline MCP branch — build_archive_pointer gained the archive_path param while its actual caller (read_cache.py:883) was left on the 3-arg form, so the most common archive path (Read skeletons) gets nothing. Rather than have you chase the wiring across both mirrored trees, we'll fold this into our own pass and credit the idea.

Heads-up separately: the CLA check is still red because the sign-off comment hasn't been posted — unrelated to the review above, just flagging so it's not a mystery.

Thanks again for the contribution and the clear write-up.

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.

2 participants