Skip to content

feat(compact): reactive compaction with direct input support - #726

Merged
leseb merged 19 commits into
praxis-proxy:mainfrom
crstrn13:feat/compact-filter-issue-30
Sep 10, 2026
Merged

feat(compact): reactive compaction with direct input support#726
leseb merged 19 commits into
praxis-proxy:mainfrom
crstrn13:feat/compact-filter-issue-30

Conversation

@crstrn13

@crstrn13 crstrn13 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Completes the remaining work items from #30:

  • Use previous_usage for token estimationshould_compact() checks the stored usage.total_tokens from the rehydrated response before falling back to local tiktoken counting
  • Configurable summary prefixsummary_prefix config option (default: [Previous conversation summary]) carried through to compaction items and both translation paths
  • Hide compaction items from input_items APInormalize_input_items() filters out {"type": "compaction"} items so clients never see internal state
  • Persist compaction as a stored response — after summarization, the compacted response is written back to the response store with the compaction item in its input
  • Explicit compact endpointPOST /v1/responses/{id}/compact triggers compaction on a previously stored response

Behavior details

  • Invalid config is rejected up front — an invalid compact_threshold (e.g. below MIN_COMPACT_THRESHOLD) returns 400 invalid_request_error before the compactability gate, for both streaming and non-streaming requests, and for direct-input requests
  • Summaries cover the full conversationshould_compact() summarizes the entire rehydrated history including the current turn, so the summarizer has complete context. The current turn is not lost or duplicated: replace_messages() preserves it verbatim after the compaction item via a tail split
  • Direct input passes through — requests with inline input (no rehydration) are not reactively compacted

Test plan

  • cargo test -p praxis-ai-apis — unit tests for all work items
  • cargo test -p praxis-ai-filters — no filter regressions
  • cargo test -p praxis-tests-integration — example integration tests (mock backends), incl. compact.rs
  • vLLM functional SDK tests (test_openai_responses_vllm.py) — invalid-threshold rejection and over-threshold rehydrated-history compaction, against live vLLM + OGX
  • make lint, rustdoc, unicode-safety, DCO/signoff all green

@crstrn13 crstrn13 changed the title feat(compact): complete remaining issue #30 items feat(compact): reactive compaction with direct input support Aug 14, 2026
@crstrn13
crstrn13 marked this pull request as ready for review August 14, 2026 13:50
@crstrn13
crstrn13 requested review from a team and jland-redhat August 14, 2026 13:50
@crstrn13 crstrn13 self-assigned this Aug 14, 2026

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Review

Summary: Completes five compaction features from #30: previous_usage token estimation fast-path, configurable summary_prefix, hiding compaction items from input_items, persisting compaction responses, and an explicit POST /v1/responses/compact endpoint.

Overall: Solid implementation with good integration tests covering both the rehydrated and direct-input paths. The summary prefix plumbing is clean and consistent across both translation paths. A few issues below around blocking in async context, stale docstrings, and missing unit test coverage for the new endpoint.

Severity Count
Large 3
Medium 3

Findings without inline placement

[Large] Stale docstrings — The struct-level doc comment on CompactFilter (~line 96 of mod.rs) says compaction "only applies to multi-turn requests where openai_responses_rehydrate has loaded stored conversation history". The generated docs page (docs/filters/openai_responses_compact.md, Configuration Notes section) says the same. Both are now incorrect — this PR adds a direct-input compaction path that runs without rehydration. Update both to match the module-level doc comment (lines 14-20), which was correctly updated.

[Large] Missing unit tests for explicit compact endpoint — The new pure/mostly-pure functions parse_compact_request_body, extract_stored_messages, is_explicit_compact_request, and ensure_compactable_state have no unit tests. Per project convention each testable function should have coverage. Suggested cases:

  • parse_compact_request_body: empty body, invalid JSON, missing response_id, valid request with optional fields
  • extract_stored_messages: empty array, non-array messages, valid array
  • is_explicit_compact_request: POST to correct path, GET to correct path, POST to wrong path
  • ensure_compactable_state: no ResponsesState, rehydrated state, non-rehydrated with compaction config, non-rehydrated without compaction config

Comment thread apis/src/openai/responses/compact/mod.rs Outdated
Comment thread apis/src/openai/responses/compact/mod.rs Outdated
Comment thread apis/src/openai/responses/compact/mod.rs
Comment thread apis/src/openai/responses/compact/mod.rs Outdated
@crstrn13
crstrn13 force-pushed the feat/compact-filter-issue-30 branch 2 times, most recently from 43ac0b3 to b0046dd Compare August 18, 2026 06:57

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-review (Aug 18 commits)

Previous review had 6 findings. The new commits addressed 4 fully (async conversion of fetch_response/build_and_persist_compaction, _body rename, response_id emptiness validation, tiktoken overhead restoration). One partially addressed (unit tests added for parse_compact_request_body and extract_stored_messages, but is_explicit_compact_request and ensure_compactable_state still lack coverage). One unaddressed (stale docstrings).

One new finding below from the rewritten explicit compact endpoint.

Severity Count
Medium 1

Comment thread apis/src/openai/responses/compact/mod.rs Outdated

@aslakknutsen aslakknutsen 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.

Outside the PR diff, but

fn should_init_store_for_request(ctx: &HttpFilterContext<'_>) -> bool {

"ResponseStoreFilter::should_init_store_for_request() returns false for POST /v1/responses/compact because the path is not is_responses_create and the classifier does not set has_previous_response_id for sub-resource routes. The store Arc is therefore never registered into ResponseStoreRegistry before CompactFilter::resolve_store_and_tenant() runs. On a freshly built pipeline (or after reload), the first explicit compact call fails with "response store not available" even when the backend is healthy. Extend should_init_store_for_request (or equivalent) to initialize the store for POST /v1/responses/compact, mirroring how rehydrate paths are handled."

Comment thread apis/src/openai/responses/compact/mod.rs
Comment thread tests/integration/tests/suite/examples/compact.rs
Comment thread apis/src/openai/responses/compact/mod.rs Outdated

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Review (re-review)

The Aug 20 commits fixed the response_object finding from the previous re-review (now includes model, created_at, and output).

One new finding from the direct-input compaction path.

Severity Count
Medium 1

Comment thread apis/src/openai/responses/compact/mod.rs Outdated

@praxis-bot praxis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-review (Aug 24 commits)

The two new commits fix the summarization scope (exclude current input from summarization text in the rehydrated path), add is_compactable unit tests, add an explicit compact endpoint integration test, fix the stale docstrings on CompactFilter and the docs page, and add store initialization for the compact endpoint.

Previous findings addressed:

  • Stale docstrings: fixed (struct doc and docs page updated)
  • Missing is_compactable / ensure_compactable_state tests: fixed (extracted is_compactable and added 5 tests)

One new finding below.

Severity Count
Medium 1

Comment thread apis/src/openai/responses/store/filter.rs Outdated
@leseb

leseb commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

@crstrn13 please rebase

@crstrn13
crstrn13 force-pushed the feat/compact-filter-issue-30 branch from b9908e8 to e849dfe Compare September 7, 2026 10:06
@praxis-bot-app

praxis-bot-app Bot commented Sep 7, 2026

Copy link
Copy Markdown

Missing Signed-off-by: e849dfe. All commits require sign-off (via git commit --signoff).

@crstrn13
crstrn13 force-pushed the feat/compact-filter-issue-30 branch from e849dfe to d2de464 Compare September 7, 2026 10:08
…roxy#30)

Signed-off-by: Alexander Cristurean <acristur@redhat.com>
@crstrn13
crstrn13 force-pushed the feat/compact-filter-issue-30 branch from f6dc948 to 359559a Compare September 7, 2026 11:40
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
…-issue-30

Signed-off-by: Alexander Cristurean <acristur@redhat.com>

# Conflicts:
#	apis/src/openai/responses/compact/mod.rs
#	tests/integration/tests/suite/examples/compact.rs
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Rephrase the module-level Scope doc so a line no longer begins with "- ",
which clippy's doc_lazy_continuation lint treated as a markdown list item
with unindented continuations. No behavior change.

Signed-off-by: Alexander Cristurean <acristur@redhat.com>
@crstrn13
crstrn13 force-pushed the feat/compact-filter-issue-30 branch from dba98b1 to 95925a4 Compare September 9, 2026 11:35
@crstrn13
crstrn13 requested a review from praxis-bot September 9, 2026 12:28
…-issue-30

Signed-off-by: Alexander Cristurean <acristur@redhat.com>

# Conflicts:
#	apis/src/openai/responses/compact/mod.rs
@crstrn13
crstrn13 requested a review from leseb September 9, 2026 15:01
- build_usage: use saturating_add for total_tokens to avoid overflow
- extract_compaction_config: reject present-but-non-array
  context_management with an error; keep absent/null as no-op.
  Extract per-entry parsing into parse_compaction_entry helper to
  stay within the line limit.
- build_and_persist_compaction: document the fail-open pass-through
  arm (usage.output_tokens is 0; caller detects the no-op via the
  absent compaction item).

Adds unit tests for null-as-absent and non-array-error cases.

Signed-off-by: Alexander Cristurean <acristur@redhat.com>
The module-level # Scope section only described reactive multi-turn
compaction and omitted POST /v1/responses/compact, which summarizes any
previously stored response regardless of rehydration. Bring it in line
with the CompactFilter struct doc, which already describes both the
reactive and explicit scenarios.

Signed-off-by: Alexander Cristurean <acristur@redhat.com>
@crstrn13
crstrn13 requested a review from a team as a code owner September 10, 2026 07:31
CI's nightly rustfmt reflows doc-comment prose to the configured
comment_width (120); the hand-wrapped bullets from the previous commit
tripped `fmt --all --check`. Reflow to match.

Signed-off-by: Alexander Cristurean <acristur@redhat.com>

@leseb leseb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

last one on_failure: open returns raw input messages inside CompactResource.output. Those lack required output fields such as type, id, and status, so typed OpenAI clients may reject the response.

…ersistence.

Signed-off-by: Alexander Cristurean <acristur@redhat.com>
@crstrn13
crstrn13 requested a review from leseb September 10, 2026 12:10
@leseb
leseb enabled auto-merge September 10, 2026 12:22

@leseb leseb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

final:

Persisted response.compaction objects have no status, but rehydration requires status == "completed". Referencing the returned ID therefore fails as status 'unknown', despite the new code claiming follow-up continuity.

Accept response.compaction records during rehydration and add an actual follow-up test.

Signed-off-by: Alexander Cristurean <acristur@redhat.com>

@leseb leseb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for your patience!

…-issue-30

# Conflicts:
#	apis/src/openai/responses/store/filter.rs
@leseb
leseb added this pull request to the merge queue Sep 10, 2026
Merged via the queue into praxis-proxy:main with commit 197494a Sep 10, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

5 participants