Skip to content

feat(chat): per-session auto-compact threshold slider in the context popover - #7346

Merged
iamwhatever merged 1 commit into
mainfrom
feat/session-autocompact-threshold
Sep 1, 2026
Merged

feat(chat): per-session auto-compact threshold slider in the context popover#7346
iamwhatever merged 1 commit into
mainfrom
feat/session-autocompact-threshold

Conversation

@helenastafford

@helenastafford helenastafford commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

The auto-compaction threshold is a single global knob (session.autocompact_pct): every session compacts at the same context-usage percentage. A long-running investigation the user wants compacted early (to keep headroom for a big synthesis step) and a short-lived session that should never compact share one value, and changing it for one session changes it for all of them.

Why it matters

Compaction timing is a per-conversation tradeoff: compact too late and a session dies mid-thought at the window ceiling; compact too early and a short session pays summarization loss it never needed. Users running heterogeneous sessions (long autonomous loops next to quick Q&A tabs) currently cannot express that difference at all.

What changed (motivation → approach → change)

Goal: let a session carry its own compaction threshold, layered over the global. Approach: mirror the established per-slot-setting pattern (slot.model endpoint + reasoning_effort persistence) rather than inventing a new mechanism, and keep the SessionManager transport-agnostic so non-dashboard surfaces can adopt it later.

  • SessionManager gains a per-key override map (CompactionState.pct_overrides) consulted by the compaction gate ladder and the warn margin (session_compaction.py). Facade methods set_autocompact_pct / effective_autocompact_pct clamp into the documented AUTOCOMPACT_PCT_MIN..MAX range (an out-of-range override degrades to the nearest firing value, never silently disables the backstop); None restores the global. The override deliberately survives reset/recycle (it is a preference on the conversation) but dies with permanent destroy() — a recreated same-key session must not silently inherit a deleted session's threshold.
  • Persistence: the dashboard slot stores the override in its open_slots metadata, re-seeds the SessionManager after restore/rehydrate (after channel-link resolution, so channel-born slots seed the session their turns actually run on), and owns the key in SLOT_OWNED_META_KEYS so a cleared override erases durably instead of resurrecting via carry_unowned_metadata.
  • API: GET/POST /api/chat/slots/{slot}/autocompact with the same validation as the global knob's PATCH handler (range, NaN, type), authorized by the session-aware ownership gate (_check_slot_app_ownership + post-await reauthorization) that the /context and /note endpoints use — stricter than the slot-only /model gate, because the write is keyed by the slot's effective session — and a log line per change.
  • Write transaction: the POST persists through the shared forced-save mechanism every slot-metadata route uses (save_slot_off_loop(force=True, best_effort=False)), pinned to the authorized transcript (expected_history_key makes the save refuse if routing moved mid-request), and serialized per transcript by an asyncio.Lock so alias slots resolving onto one file cannot interleave. After the commit, the value is mirrored to live alias siblings and re-confirmed with a second pinned save, so a queued sibling flush cannot durably revert the acknowledged value; every failure path rolls live state back (409 on delete/rebind, 500 with reconvergence on I/O failure). Legacy transcripts without created_at stay covered by the delete-won guard via an observed bit recorded at every hydration site (pinned by a structural census test).
  • UI: the ChatInput context-usage popover gains a slider section (design-system Slider): value readout, a fixed sense of the global via the reset link ("Reset to global (N%)") when overridden, and a "Following global (N%)" note otherwise. Fetched lazily on popover open through React Query (['slot-autocompact', slot]); slider drags write optimistically into the query cache and collapse into one debounced POST, with all write paths (debounce, cross-slot flush, unmount flush) ordered through a per-slot promise chain. Loading renders skeleton rows; a failed fetch renders a muted explanation (suppressed while a cached value still renders the slider); a rejected write surfaces the same visible failure notice the model switcher uses.
  • i18n: 5 keys across English + 11 locales (vocabulary matched to each locale's existing "Auto-Compact Threshold" setting term) + regenerated en-XA.

Out of scope (deliberate): non-dashboard transports (Slack/Discord), the task runner, and CLI chat keep the global threshold; the override map is transport-agnostic so those can be wired later without redesign.

Tests

  • test/test_session_autocompact_override.py (59 tests):
    • Override moves the gate for its own session only; sibling sessions keep the global.
    • Clamp to range; NaN ignored; None restores global; clearing an absent override is a no-op.
    • Endpoint: GET reports override/global/range; POST sets slot + live SessionManager + dirty flag; null clears; invalid values (out-of-range, NaN, string, bool, list, missing key) rejected 400; unknown slot 404.
    • Persistence validator: round-trip, clamp, garbage discarded.
    • Lifecycle (both revert-verified — they fail without the fix): a cleared override does not resurrect through carry_unowned_metadata (autocompact_pctSLOT_OWNED_META_KEYS), and destroy() clears the override.
    • Transaction hardening (revert-verified): transcript-keyed lock serialization for alias slots; alias-sibling mirror with post-mirror confirm-save ordering and rollback; a mid-persist rebind returns 409 without seeding the foreign session's live gate; the expected_history_key pin refuses a moved save; legacy (no created_at) delete-won coverage, pinned by a structural census of every _disk_meta_created_at hydration site; a slotless permanent delete of archived history sweeps the override (fold-matching, same contract as the session-ledger purge), so a recreated deterministic-key session cannot inherit a deleted conversation's threshold.
  • ChatInputCoverage.test.tsx: slider render/override/reset/following-global; write ordering through the per-slot promise chain including cross-slot and unmount flush; loading skeleton; failed-fetch explanation (suppressed while a cached value still renders); a rejected write surfaces a visible failure notice.

Manual verification

Rendered the real built SPA against a stubbed dashboard API via the committed capture harness (website/scripts/capture-autocompact-slider.mjs) and pixel-verified both states against the approved mockup (see screenshots). Backend regression suites for compaction/destroy paths (63 tests) and the adjacent slot-endpoint suites pass.

Screenshots / video

Session at 72% context, global threshold 70%.

Override set (85%) — reset link visible:

Context popover with the auto-compact slider set to a session override of 85% and a reset-to-global link

No override — following the global:

Context popover with the slider following the global 70% threshold

Related Issues

no linked issue: feature implemented directly from a maintainer-visible design discussion (slider option chosen from three mocked alternatives); no tracking issue was filed.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable) — N/A: no docs surface documents the global knob's UI today
  • No secrets, credentials, or internal references in the diff

@helenastafford
helenastafford requested a review from a team August 31, 2026 19:38
@helenastafford
helenastafford requested a review from a team as a code owner August 31, 2026 19:38
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 352ef5b68440536cad66655e93d0a2f0436aec7e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have enough to render the design review. The backend override design is sound (mirrors the model/reasoning_effort pattern); the real findings are spec drift against two module specs, a shared-guard behavior change bundled in, and committed binaries.

Design-Verdict: CONCERNS

Sound per-session override design, but it changes two spec-documented behaviors with zero spec updates, and bundles a global save-guard change plus repo binaries.

Watch

  • Spec drift, twice. docs/system-specs/modules/session.md:197 documents compaction as firing at the single global session.autocompact_pct — this PR makes that consult a per-session override first. docs/system-specs/modules/history.md (~L246–265) documents the delete-won guard as "a readable-but-absent created_at (legacy meta) fails open" — the new _disk_meta_observed bit changes exactly that. AGENTS.md requires spec updates in the same commit; the checklist's "docs N/A" claim is wrong. Consequence: the next contributor reasoning from either spec designs against behavior that no longer exists.
  • The delete-won widening is an unrelated shared-path change. _disk_meta_observed alters save/deletion semantics for every legacy-transcript save (a missing file now refuses the save instead of recreating it), riding in a slider feature PR. Real fix, wrong vehicle — split it out so it can be bisected/reverted independently of the feature.
  • temp-screenshots/ commits ~355KB of PNGs into permanent git history, in a root directory literally named "temp", solely to back PR-description image links. Attach them to the PR instead and drop the directory (and decide whether the one-off capture-autocompact-slider.mjs harness earns a permanent home).

Suggestions

  • The lock → pin → mirror → confirm-save → rollback choreography is generic to any slot-owned metadata write (model/note/tags share the alias-slot stale-flush race), yet lives as ~200 endpoint-local lines; extract a shared transactional-metadata-write helper before a second key copies it.

[DESIGN-REVIEWED] 352ef5b

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 352ef5b68440536cad66655e93d0a2f0436aec7e — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. Producing the review.

First-Principles-Verdict: CONCERNS

One preference slider ships ~150 lines of bespoke write-transaction machinery guarding races that ten counted sibling metadata writes live with untouched.

What this change ships

Intent: let each session set its own auto-compact threshold instead of one global value — an ADDITION.

  1. Slider in the context popover sets this session's compaction threshold — justified
  2. "Reset to global" / "Following global" affordance — justified
  3. New GET/POST /api/chat/slots/{slot}/autocompact — justified (mirrors the per-slot /model pattern)
  4. Override persists with the slot, survives restart/reset, dies on permanent delete — justified
  5. Override sweep on slotless history delete — justified (deterministic channel keys)
  6. Legacy transcripts (no created_at) now get delete-won protection on every save path (_disk_meta_observed) — declared rider; general fix in a feature PR
  7. expected_history_key refuse-if-moved pin on the shared save — one consumer (both calls in the one new handler)
  8. Per-transcript lock + alias-sibling mirror + second confirm-save on the POST — oversized
  9. 5 i18n keys × 12 locales — justified (documented invariant)
  10. Capture harness + committed screenshots — justified (PR-template convention; 330 sibling scripts)

Watch

  • Items 7–8 defend against interleavings generic to every slot-owned field: grep save_slot_off_loop(state, slot, force=True → 10 sibling mutate-then-persist sites (tags ×3, folders ×5, slot-create ×1, auto-tag ×1) with no pin, no lock, no mirror; higher-stakes /model (chat_handlers.py:4574) settles for an in-memory lock + CAS rollback. Either the races are tolerable for a percent too, or the cause — _save_slot_to_history writing every field from the caller's in-memory copy — belongs in the shared save layer where all ten sites inherit the fix. As shipped this is a point-hardened endpoint whose guarantees silently diverge from every sibling's.
  • Item 6 changes save/delete semantics for all transcripts, not just this feature's field; declared, and needed for the endpoint's 409 on legacy files, but its blast radius exceeds the feature — worth a human eye.

Subtractions

  • Shrink api_chat_slot_autocompact to the /model shape (validate → reauth → set field → one forced save → seed live map, per-slot in-memory lock): drop _autocompact_txn_locks, the alias-mirror loop, the second confirm-save, and the expected_history_key parameter — or land the pin/mirror inside save_slot_off_loop so the 10 counted sibling sites get it too.

[FIRST-PRINCIPLES-REVIEWED] 352ef5b

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

UX-level review of 352ef5b68440536cad66655e93d0a2f0436aec7e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The slider reuses the established Slider primitive (full keyboard support, aria-label), failure feedback routes through the same notice channel the model/reasoning-effort switchers use, states are complete (skeleton, error, override/following), and both screenshots match the product's dense popover style and the PR's claims. No material UX risk found.

UX-Verdict: PASS

Clear layered disclosure — "Auto-compact at" + "Following global (N%)" / "Reset to global (N%)" makes the override model self-evident, with complete loading/error/failure states.

[UX-REVIEWED] 352ef5b

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 352ef5b68440536cad66655e93d0a2f0436aec7e — this comment is updated in place on each push.

Review details

Both candidates fail the Step 1 bar under falsification.

Candidate 1 (mirror-rollback re-derives membership): The alleged leak requires three events to coincide in a sub-millisecond window — an alias sibling on the same transcript, that sibling's linked_session_key being re-pointed to a foreign session during the confirm-save await, and the confirm save then failing or refusing. The candidate itself concedes it "could not construct the precise interleaving." The trigger is a "could"/"if a caller were to" condition, not a concrete input that occurs in practice, so (a) is not re-derivable at 80+. The forward mirror and the rollback both deliberately re-derive membership (documented invariant: a rebound sibling "no longer writes this file" and must not receive a live change its reauthorization is about to deny), so the symmetry is by design, not an oversight. Dropped.

Candidate 2 (committed temp-screenshots/*.png): A repo-hygiene concern with no observable runtime defect — nothing the code does when executed changes. It is not any reportable class here (not a crash, data loss, corruption, security hole, or removed guard), and the candidate rates it "low," unsure whether it is intentional. Dropped.

No new grounded finding surfaced at the required bar.

No findings.

[OPUS-REVIEWED] 352ef5b

Verdict parsed from the review's SHA-scoped output markers for commit 352ef5b68440536cad66655e93d0a2f0436aec7e.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 352ef5b68440536cad66655e93d0a2f0436aec7e: <one-sentence reason>

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 352ef5b68440536cad66655e93d0a2f0436aec7e and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 352ef5b

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 352ef5b68440536cad66655e93d0a2f0436aec7e: <one-sentence reason>

@helenastafford
helenastafford force-pushed the feat/session-autocompact-threshold branch 4 times, most recently from 4e5ee0b to 8561222 Compare August 31, 2026 21:32
@helenastafford

helenastafford commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author
  • fixed — non-object JSON crashes the endpoint (chat_handlers.py)

Verified reachable: "pct" not in body raised TypeError on JSON null/number/string bodies, and float(10**400) raised uncaught OverflowError — both HTTP 500. Fixed in 85612222d by rejecting non-dict bodies with 400 before the membership test and catching OverflowError in the numeric conversion. Revert-verified regression tests added (test_post_non_object_json_body_is_400_not_500, test_post_huge_int_is_400_not_500): they fail with 500s without the fix, pass with it.

Posted by Ember, helenars's AI agent.

@helenastafford
helenastafford force-pushed the feat/session-autocompact-threshold branch from 8561222 to 9073b42 Compare August 31, 2026 21:37
@helenastafford

Copy link
Copy Markdown
Collaborator Author

Disposition for the Opus 4.8 advisory on 33555a0a2; addressed in 9073b4230.

  • Non-2xx bodies missing a machine-readable code field (chat_handlers.py autocompact endpoint) — fixed

Verified against AGENTS.md and the sibling handlers (which emit "code": "slot_not_found" etc.). Every non-2xx body on GET/POST /api/chat/slots/{slot}/autocompact now carries a stable code: slot_not_found, invalid_json, pct_required, pct_not_a_number, pct_not_finite, pct_out_of_range. A parametrized test (test_error_bodies_carry_machine_readable_codes) locks in the code contract for each error path.

Posted by Ember, helenars's AI agent.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 31, 2026
@helenastafford
helenastafford force-pushed the feat/session-autocompact-threshold branch from 9073b42 to 65a6662 Compare August 31, 2026 22:06
@helenastafford

Copy link
Copy Markdown
Collaborator Author

Disposition for the GPT 5.6 Review round on 9073b4230; addressed in 65a666266.

  • Oversized persisted threshold aborts gateway startup (chat_persistence.py _validate_autocompact_pct) — fixed

Verified: float(raw) on a huge persisted int raised uncaught OverflowError on the recent-session restore path. The validator now catches OverflowError and discards the value with a warning, matching its NaN handling. This was the sibling branch of the round-2 endpoint OverflowError fix, so the whole class was swept: the third float() conversion site on the override path (the SessionManager facade clamp in session.py, reachable via set_autocompact_pct) got the same guard, ignoring oversized values like NaN. Revert-verified regression tests added for both sites (test_huge_int_is_discarded_not_crash, test_huge_int_is_ignored_not_raised): both fail without the fixes, pass with them. No other unguarded float() conversions remain in the PR diff.

Posted by Ember, helenars's AI agent.

@helenastafford
helenastafford force-pushed the feat/session-autocompact-threshold branch from 65a6662 to a099878 Compare August 31, 2026 22:43
@helenastafford

helenastafford commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author
  • fixed — empty-session overrides are never persisted (chat_handlers.py)

Verified: _save_slot_to_history returns before any write when the message window is empty (if not window: return True precedes the force check), so slot._dirty = True on a message-less slot never reached disk and a restart restored the global threshold. Fixed in a09987825: the endpoint also writes the metadata directly via conversation_log.update_metadata in an asyncio.to_thread (the pattern update_metadata_off_loop's docstring prescribes for async handlers), keeping the dirty flag as the retry path. Revert-verified regression test (test_post_writes_metadata_directly_for_empty_slots) fails without the fix.

Posted by Ember, helenars's AI agent.

@helenastafford
helenastafford force-pushed the feat/session-autocompact-threshold branch from a099878 to a57bef1 Compare August 31, 2026 23:30
@helenastafford

Copy link
Copy Markdown
Collaborator Author

fixed — ChatInput.tsx: switching sessions could discard a pending threshold write.

The debounce clearTimeout dropped the pending write unconditionally, so drag on session A → switch to B → drag on B within 400ms silently kept A's old threshold on the server. Fixed in a57bef1766c9a251a8992ac9dfbb668bdbab52bf: debouncing now only supersedes same-slot writes — a pending write whose slot differs from the incoming write's slot is flushed immediately via the mutation before the new debounce is armed. Revert-verified regression test added ('flushes a pending write for another slot when a new slot writes within the debounce window'): fails without the fix, passes with it.

Both round-5 findings are scoped to the round-4 delta (the unmount-flush sibling branch and the ordering of the round-4 metadata write), so this is a targeted delta fix, not the already-reviewed-surface escalation case declared in the round-4 disposition. That escalation condition remains armed for round 6.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author

fixed — chat_handlers.py: persistence failure left an unacknowledged live threshold.

The handler mutated slot.autocompact_pct and the SessionManager override before the direct metadata write, so an I/O failure returned 500 while the live threshold had already changed — an unacknowledged change a restart would then silently revert. Fixed in a57bef1766c9a251a8992ac9dfbb668bdbab52bf: the metadata write now runs FIRST inside a try/except; on failure the handler returns a coded 500 (persist_failed) with slot field, override map, and _dirty all untouched. Live state mutates only after the persist succeeds. Revert-verified regression test added (test_persist_failure_leaves_live_state_untouched): fails without the reorder, passes with it.

Posted by Ember, helenars's AI agent.

@helenastafford
helenastafford force-pushed the feat/session-autocompact-threshold branch from a57bef1 to cd18467 Compare August 31, 2026 23:56
@helenastafford

Copy link
Copy Markdown
Collaborator Author

fixed — chat_handlers.py: linked app-owned slots could bypass session authorization.

Verified real: the POST writes the override keyed by effective_session_key(slot), so an app naming an existing channel stem owned a slot bound to a session it has no claim on, and the slot-only gate (_deny_cross_app_slot_access) passed it. Fixed in cd18467678d00f56634cebc28dda31ffdf22de44: the endpoint now uses _check_slot_app_ownership (which authorizes the session and transcript keys the write actually lands on, per its documented discipline) plus _reauthorize_after_await before the first write, mirroring /context and /note. Revert-verified regression test added (test_app_token_on_linked_slot_is_denied): without the fix the app's POST returned 200 and modified the foreign session; with it, a byte-identical anti-enumeration 404.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author

fixed — ChatInput.tsx: rejected unmount flush left the optimistic threshold cached.

Advisory FINDING, fixed anyway (one line, same invariant as the mounted path's onError): the unmount flush's rejection handler now invalidates ['slot-autocompact', pending.slot] so a return to the slot refetches server truth instead of showing a threshold the session never applied. In cd18467678d00f56634cebc28dda31ffdf22de44. No dedicated test: the invalidate-on-rejection contract is already locked by the mounted-path regression test ('refetches the applied threshold when a write is rejected'), and this handler is the same one-line pattern applied to the flush path.

Posted by Ember, helenars's AI agent.

@helenastafford
helenastafford force-pushed the feat/session-autocompact-threshold branch from cd18467 to a34e203 Compare September 1, 2026 00:18
@helenastafford

Copy link
Copy Markdown
Collaborator Author

fixed — chat_handlers.py: the persist await reopened the session-authorization race.

Verified real: the round-5 metadata await sat between the round-6 reauthorization and the live mutations, so a slot rebind during the persist landed the override on a foreign session one await later. Fixed in a34e2030eb7f105244e4ea3c29d464aa315ff546 by the structural invariant, not another point patch — SPAN NOTE: this is the 3rd consecutive blocking round in chat_handlers.py:api_chat_slot_autocompact (rounds 5, 6, 7), so per the same-span rule the fix is the invariant that makes the whole class unreachable: every write in this handler is immediately preceded by an authorization decision with no await between them. Write-site table: (1) persist write — gated by the post-body-read _reauthorize_after_await, its key computed synchronously after that gate; (2) live mutations (slot field, SessionManager override, dirty flag) — gated by a second _reauthorize_after_await placed after the persist await, inside the same conditional so the no-persist branch (no await) correctly needs no second gate; (3) GET — reads only, entry gate suffices. The persisted metadata written before a mid-persist rebind is keyed by the pre-await transcript key and is inert (its restore path re-validates). Revert-verified test test_rebind_during_persist_await_is_denied: the mock rebinds linked_session_key from inside update_metadata; without the fix the POST returned 200 and mutated the foreign session, with it a byte-identical anti-enumeration 404 and zero mutations. Any future await added to this handler must carry its own reauth per the inline INVARIANT comment.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — alias slots sharing one transcript could revert an acknowledged commit

Fixed in bd1b923. The transaction lock is now keyed by the TRANSCRIPT (_autocompact_txn_lock(slot_history_key(slot))), so alias slots resolving onto one file serialize on one lock; the lock key is re-verified after acquisition (409 on a rebind between read and acquire). After the durable commit, the value is mirrored to every live slot whose current history key matches the authorized transcript (membership re-derived post-persist, priors recorded), and a second pinned confirmed save re-orders the durable record after any sibling flush the file lock interleaved — so a queued sibling's stale-field flush can no longer durably revert the commit. Regression tests (revert-verified): TestAliasSlotsShareOneTranscript, TestSiblingFlushCannotStaleTheCommit (ordering proven by capturing the sibling field at each save: stale at commit, mirrored at confirm).

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — legacy metadata without created_at skipped the delete-won guard

Fixed in bd1b923. Hydration observation is now tracked as its own bit (_disk_meta_observed) instead of being inferred from a non-empty created_at, recorded at EVERY hydration site — both persistence loaders, the committed-save path, the resume endpoint, and channel surfacing — and the guard's evidence gate widened to if _known or slot._disk_meta_observed: with the identity comparison still requiring a recorded created_at on both sides (so a sibling stamping created_at into a live legacy file cannot false-positive a refusal). A structural census test (TestObservedBitRecordedAtEveryHydrationSite) pins all current and future _disk_meta_created_at recording sites under src/kiro_crew/dashboard/. Regression tests (revert-verified): TestLegacyMetadataDeleteWon — a deleted legacy transcript is not resurrected; an existing legacy file still saves.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — zero-consumer SessionManager facades deleted (subtraction adopted)

Adopted in bd1b923, reversing the earlier keep-the-facade position: SessionManager.effective_autocompact_pct and the autocompact_pct_override wrapper had no non-test consumers (verified by grep across src/), so both were deleted. Callers and tests now read through the compaction coordinator directly (mgr._compaction.effective_autocompact_pct(...), which folds keys itself); set_autocompact_pct remains as the single write facade because the endpoint and hydration re-seed paths consume it.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • rebutted — the per-transcript transaction lock is not removable apparatus

The proposal to delete the endpoint's transaction lock does not hold: serialization is a distinct concern from the deleted persistence apparatus, and the shared best-effort save provides no endpoint-scoped rollback or ordering semantics. The GPT lane's verified alias-slot blocker on this same head is the concrete proof — a lock keyed by the SLOT (the narrower shape) already failed to serialize two alias slots interleaving one transcript's write span, so the class needs a lock keyed by the transcript, not no lock. The round-16 concurrency tests (TestConcurrentPostSerialization, test_failed_request_cannot_erase_a_committed_sibling) fail without it.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • accepted-and-deferred — pin expected_history_key at the remaining forced-save call sites (tags/folders)

Correct observation, deliberately out of this PR's scope: the sibling forced-save call sites (tags, folders, pin) predate this PR and widening them here would grow the diff beyond the autocompact feature. Filed as issue #7519 with the call-site list and the pin recipe this PR established, so it is an actionable task, not a parked question.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — PR body described the authorization gate as the /model gate; the code uses the stricter session-aware gate

Prose-only fix, applied to the PR body alongside the bd1b923 push: the description now states the endpoint authorizes via the session-aware ownership gate (_check_slot_app_ownership + post-await reauthorization, the /context//note pattern) rather than the slot-only /model gate, matching what the code has done since round 6.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — a rejected threshold write was silent

Fixed in bd1b923: the mutation's onError and the unmount-flush rejection both dispatch the same visible failure notice the model switcher uses (setAgentSwitchNotice(agentSwitchFailureMessage(err))), so a 409/500 on the slider is surfaced instead of silently reverting. Regression test: "surfaces a rejected write as a visible notice" (revert-verified — fails without the dispatch).

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — the popover section popped in with no loading state

Fixed in bd1b923: while the threshold query is loading, the section renders two skeleton rows (aria-hidden, pulse animation) matching the section's final footprint, so the popover does not reflow when data arrives.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — a failed threshold fetch silently omitted the section

Fixed in bd1b923: a failed fetch now renders a muted explanation line (new components.chatInput.auto_compact_load_failed key across en.manual + 11 locales, each using its locale's established vocabulary, + regenerated en-XA). Per the Opus local-review advisory, the line is suppressed while a cached value still renders the slider, so the error text can never contradict an interactive control. Regression test: "explains a failed threshold fetch".

Posted by Ember, helenars's AI agent.

…popover

The compaction threshold (session.autocompact_pct) was global-only: one
value governs every session, so a long investigation that should compact
early and a short session that should never compact share one knob.

This adds a per-session override layered over the global:

- SessionManager gains a per-key override map (CompactionState.pct_overrides)
  consulted by the gate ladder and the warn margin; values clamp into the
  documented AUTOCOMPACT_PCT_MIN..MAX range and None restores the global.
- The dashboard slot persists the override (open_slots metadata) and re-seeds
  the SessionManager after restore/rehydrate, after channel-link resolution.
- GET/POST /api/chat/slots/{slot}/autocompact exposes it with the same
  validation as the global knob's PATCH handler.
- The ChatInput context popover gains a slider section (design-system Slider):
  value readout, reset-to-global link when overridden, following-global note
  otherwise. Lazy-fetched on popover open; POSTs are debounced.
- i18n: 4 keys across en + 11 locales + en-XA.

Non-dashboard transports (Slack/Discord/task runner/CLI) keep the global
threshold in this change.
@helenastafford
helenastafford force-pushed the feat/session-autocompact-threshold branch from bd1b923 to 352ef5b Compare September 1, 2026 06:38
@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — slotless permanent deletion retained a deleted session's override

Fixed in 352ef5b. destroy() clears a live session's override, but the history-delete helper only reaches it when a live slot exists — deleting ARCHIVED history left pct_overrides holding the deleted key, and channel keys are deterministic, so a recreated session inherited the dead conversation's threshold. Added drop_autocompact_overrides_matching on the compaction coordinator (public SessionManager facade for the handler), invoked from _remove_slot_for_history_key after the ledger sweep with the same fold-matching contract (exact + folded spellings of the deleted key) — covering both the single-delete and bulk-clear endpoints, which share that helper. Regression tests (revert-verified): the mechanism drops exact and folded matches while preserving unrelated overrides, and the slotless-delete wiring test asserts the sweep runs with the deleted key when no slot exists and destroy() is unreachable.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — failed writes leave a false threshold cached (ChatInput.tsx)

Verified: the debounced optimistic setQueryData had no error rollback, so a rejected POST left the UI showing a threshold the session never applied. Fixed in 85612222d with an onError handler that invalidates ['slot-autocompact', slot], refetching server truth so the slider snaps back to the applied value. Regression test added: a rejected write triggers a refetch and the pre-write value is displayed.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — function-local loader import (chat_persistence.py)

The advisory top-level-imports finding: the module already imports from kiro_crew.config.loader at top level, so the function-local import had no circularity justification. Fixed in 85612222d by moving AUTOCOMPACT_PCT_MIN/MAX into the existing top-level import.

Posted by Ember, helenars's AI agent.

@helenastafford

Copy link
Copy Markdown
Collaborator Author
  • fixed — unmounting discards the pending threshold change (ChatInput.tsx)

Verified: the unmount cleanup cleared the debounce timer without flushing, cancelling the sole POST when the user navigated away within 400ms of a slider change. Fixed in a09987825: the cleanup now flushes the pending write directly through the API client (the component is gone, so the mutation's cache re-sync has nothing to update). Revert-verified regression test (flushes a pending debounced write on unmount) fails without the fix.

Posted by Ember, helenars's AI agent.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention labels Sep 1, 2026
@iamwhatever
iamwhatever merged commit 275a2e5 into main Sep 1, 2026
76 of 78 checks passed
@iamwhatever
iamwhatever deleted the feat/session-autocompact-threshold branch September 1, 2026 18:53
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 1, 2026
chenmingwei23 pushed a commit that referenced this pull request Sep 1, 2026
…s forced saves (#7519)

save_slot_off_loop resolves its target transcript from live routing at
write time, so a linked_session_key rebind during the persist await could
redirect a durable write to a transcript the caller never authorized
against. PR #7346 added the expected_history_key refuse-if-moved pin and
wired it at the autocompact endpoint only.

Thread the same pin through the remaining tags/folders forced-save sites:

- chat_tags: the tag-delete slot strip, PUT slot tags, and the drag-drop
  status reassign.
- chat_folders: the folder-delete unfile loop and its restore rollback,
  PATCH slot folder, PATCH slot pin, and PATCH slot mode.

The five request endpoints capture the authorized key BEFORE their first
await and re-check it (plus slot object identity) after the last await
before mutating, mirroring the reauthorize-then-capture shape of the
autocompact precedent. Each site handles the refusal per its own
convention: the direct mutation endpoints roll back and return 409
session_gone, the drag-drop endpoint answers in its own ok:false
rejection shape, and the best-effort cleanup loops mark the slot dirty
for the periodic flush and keep going. Rollbacks are compare-and-set (a
concurrent writer's acknowledged commit is never erased) and restore the
prior _folder_changed latch rather than clearing it.

Same-class force=True sites outside this issue's tags/folders scope
(slot-recreate in chat_handlers, chat_auto_tag, crew_chat) are left for
a follow-up.

Closes #7519
chenmingwei23 pushed a commit that referenced this pull request Sep 1, 2026
…s forced saves (#7519)

save_slot_off_loop resolves its target transcript from live routing at
write time, so a linked_session_key rebind during the persist await could
redirect a durable write to a transcript the caller never authorized
against. PR #7346 added the expected_history_key refuse-if-moved pin and
wired it at the autocompact endpoint only.

Thread the same pin through the remaining tags/folders forced-save sites:

- chat_tags: the tag-delete slot strip, PUT slot tags, and the drag-drop
  status reassign.
- chat_folders: the folder-delete unfile loop and its restore rollback,
  PATCH slot folder, PATCH slot pin, and PATCH slot mode.

The five request endpoints capture the authorized key BEFORE their first
await and re-check it (plus slot object identity) after the last await
before mutating, mirroring the reauthorize-then-capture shape of the
autocompact precedent. Each site handles the refusal per its own
convention: the direct mutation endpoints roll back and return 409
session_gone, the drag-drop endpoint answers in its own ok:false
rejection shape, and the best-effort cleanup loops mark the slot dirty
for the periodic flush and keep going. Rollbacks are compare-and-set (a
concurrent writer's acknowledged commit is never erased) and restore the
prior _folder_changed latch rather than clearing it.

Same-class force=True sites outside this issue's tags/folders scope
(slot-recreate in chat_handlers, chat_auto_tag, crew_chat) are left for
a follow-up.

Closes #7519
chenmingwei23 pushed a commit that referenced this pull request Sep 1, 2026
…s forced saves (#7519)

save_slot_off_loop resolves its target transcript from live routing at
write time, so a linked_session_key rebind during the persist await could
redirect a durable write to a transcript the caller never authorized
against. PR #7346 added the expected_history_key refuse-if-moved pin and
wired it at the autocompact endpoint only.

Thread the same pin through the remaining tags/folders forced-save sites:

- chat_tags: the tag-delete slot strip, PUT slot tags, and the drag-drop
  status reassign.
- chat_folders: the folder-delete unfile loop and its restore rollback,
  PATCH slot folder, PATCH slot pin, and PATCH slot mode.

The five request endpoints capture the authorized key BEFORE their first
await and re-check it (plus slot object identity) after the last await
before mutating, mirroring the reauthorize-then-capture shape of the
autocompact precedent. Each site handles the refusal per its own
convention: the direct mutation endpoints roll back and return 409
session_gone, the drag-drop endpoint answers in its own ok:false
rejection shape, and the best-effort cleanup loops mark the slot dirty
for the periodic flush and keep going. Rollbacks are compare-and-set (a
concurrent writer's acknowledged commit is never erased) and restore the
prior _folder_changed latch rather than clearing it.

Same-class force=True sites outside this issue's tags/folders scope
(slot-recreate in chat_handlers, chat_auto_tag, crew_chat) are left for
a follow-up.

Closes #7519
chenmingwei23 pushed a commit that referenced this pull request Sep 1, 2026
…s forced saves (#7519)

save_slot_off_loop resolves its target transcript from live routing at
write time, so a linked_session_key rebind during the persist await could
redirect a durable write to a transcript the caller never authorized
against. PR #7346 added the expected_history_key refuse-if-moved pin and
wired it at the autocompact endpoint only.

Thread the same pin through the remaining tags/folders forced-save sites:

- chat_tags: the tag-delete slot strip, PUT slot tags, and the drag-drop
  status reassign.
- chat_folders: the folder-delete unfile loop and its restore rollback,
  PATCH slot folder, PATCH slot pin, and PATCH slot mode.

The five request endpoints capture the authorized key BEFORE their first
await and re-check it (plus slot object identity) after the last await
before mutating, mirroring the reauthorize-then-capture shape of the
autocompact precedent. Each site handles the refusal per its own
convention: the direct mutation endpoints roll back and return 409
session_gone, the drag-drop endpoint answers in its own ok:false
rejection shape, and the best-effort cleanup loops mark the slot dirty
for the periodic flush and keep going. Rollbacks are compare-and-set (a
concurrent writer's acknowledged commit is never erased) and restore the
prior _folder_changed latch rather than clearing it.

Same-class force=True sites outside this issue's tags/folders scope
(slot-recreate in chat_handlers, chat_auto_tag, crew_chat) are left for
a follow-up.

Closes #7519
chenmingwei23 pushed a commit that referenced this pull request Sep 2, 2026
…s forced saves (#7519)

save_slot_off_loop resolves its target transcript from live routing at
write time, so a linked_session_key rebind during the persist await could
redirect a durable write to a transcript the caller never authorized
against. PR #7346 added the expected_history_key refuse-if-moved pin and
wired it at the autocompact endpoint only.

Thread the same pin through the remaining tags/folders forced-save sites:

- chat_tags: the tag-delete slot strip, PUT slot tags, and the drag-drop
  status reassign.
- chat_folders: the folder-delete unfile loop and its restore rollback,
  PATCH slot folder, PATCH slot pin, and PATCH slot mode.

The five request endpoints capture the authorized key BEFORE their first
await and re-check it (plus slot object identity) after the last await
before mutating, mirroring the reauthorize-then-capture shape of the
autocompact precedent. Each site handles the refusal per its own
convention: the direct mutation endpoints roll back and return 409
session_gone, the drag-drop endpoint answers in its own ok:false
rejection shape, and the best-effort cleanup loops mark the slot dirty
for the periodic flush and keep going. Rollbacks are compare-and-set (a
concurrent writer's acknowledged commit is never erased) and restore the
prior _folder_changed latch rather than clearing it.

Same-class force=True sites outside this issue's tags/folders scope
(slot-recreate in chat_handlers, chat_auto_tag, crew_chat) are left for
a follow-up.

Closes #7519
iamwhatever pushed a commit that referenced this pull request Sep 2, 2026
…s forced saves (#7519) (#7714)

save_slot_off_loop resolves its target transcript from live routing at
write time, so a linked_session_key rebind during the persist await could
redirect a durable write to a transcript the caller never authorized
against. PR #7346 added the expected_history_key refuse-if-moved pin and
wired it at the autocompact endpoint only.

Thread the same pin through the remaining tags/folders forced-save sites:

- chat_tags: the tag-delete slot strip, PUT slot tags, and the drag-drop
  status reassign.
- chat_folders: the folder-delete unfile loop and its restore rollback,
  PATCH slot folder, PATCH slot pin, and PATCH slot mode.

The five request endpoints capture the authorized key BEFORE their first
await and re-check it (plus slot object identity) after the last await
before mutating, mirroring the reauthorize-then-capture shape of the
autocompact precedent. Each site handles the refusal per its own
convention: the direct mutation endpoints roll back and return 409
session_gone, the drag-drop endpoint answers in its own ok:false
rejection shape, and the best-effort cleanup loops mark the slot dirty
for the periodic flush and keep going. Rollbacks are compare-and-set (a
concurrent writer's acknowledged commit is never erased) and restore the
prior _folder_changed latch rather than clearing it.

Same-class force=True sites outside this issue's tags/folders scope
(slot-recreate in chat_handlers, chat_auto_tag, crew_chat) are left for
a follow-up.

Closes #7519

Co-authored-by: Raymond Chen <bolichen97@users.noreply.github.com>
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