Skip to content

docs(api): normalize source docstring backticks#1155

Merged
planetf1 merged 2 commits into
generative-computing:mainfrom
planetf1:fix/rst-backticks-657
May 27, 2026
Merged

docs(api): normalize source docstring backticks#1155
planetf1 merged 2 commits into
generative-computing:mainfrom
planetf1:fix/rst-backticks-657

Conversation

@planetf1

@planetf1 planetf1 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Type of PR

  • Bug Fix
  • Documentation
  • New Feature
  • Other

Description

Replace RST-style ``symbol`` notation with Markdown-style `symbol` notation in Python string literals under mellea/ and cli/. Keeps fenced code blocks (triple backticks) intact.

This is a fresh branch off current main, superseding PR #659 (closed) which could not be rebased cleanly after 206 intervening commits.

Note: This is optional cleanup — the docs pipeline already normalises these at generation time (#658). The alternative is to close this PR and formally permit RST inline-code notation in docstrings. This is left to maintainer discretion.

Scope

This PR is intentionally narrow: it fixes only the double-backtick inline-code pattern ( symbol ). A separate audit found 12 files with ~89 additional RST-style constructs (:param, :type, :returns:, :rtype:, .. note::, .. deprecated::) concentrated in mellea/formatters/granite/ and mellea/stdlib/components/intrinsic/. Those require editorial restructuring into Google-style docstrings and are out of scope here.

Changes

  • 127 files, 1977 substitutions: inline ``x```x` in docstrings across mellea/ and cli/
  • One pre-existing malformed docstring in session.py also corrected (missing opening backtick)
  • Fenced code blocks (```python / ```) left untouched
  • All Python files compile cleanly (python -m compileall)
  • ruff check and ruff format pass

Merge timing suggestion

Because this touches 127 files it creates a wide conflict surface. Suggest merging shortly after a release cut so the docs pipeline can be verified against a clean baseline before the next batch of feature work lands on top.

Testing

  • python -m compileall mellea/ cli/ passes
  • ruff check mellea/ cli/ passes
  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code was added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

@planetf1

Copy link
Copy Markdown
Contributor Author

During the audit for this PR, we found 12 files with ~89 RST-style :param, :type, :returns:, :rtype:, .. note::, and .. deprecated:: constructs — mostly in mellea/formatters/granite/ and mellea/stdlib/components/intrinsic/. These are out of scope here (they need editorial restructuring into Google-style docstrings, not a mechanical substitution).

Reviewer question: should we track this as a follow-up issue, or is the current mixed style acceptable given that code is largely stable?

@planetf1
planetf1 force-pushed the fix/rst-backticks-657 branch from 3d64cd2 to c0b0fa3 Compare May 27, 2026 08:15
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 27, 2026
@planetf1
planetf1 marked this pull request as ready for review May 27, 2026 08:20
@planetf1
planetf1 requested review from a team, jakelorocco and nrfulton as code owners May 27, 2026 08:20
@planetf1
planetf1 requested a review from psschwei May 27, 2026 08:20
Comment on lines -72 to +76
# Pattern for ```python ... ``` blocks
python_blocks = re.findall(r"```python\s*\n(.*?)\n```", content, re.DOTALL)
# Pattern for ``python ... `` blocks
python_blocks = re.findall(r"``python\s*\n(.*?)\n``", content, re.DOTALL)

# Pattern for generic ``` blocks
generic_blocks = re.findall(r"```\s*\n(.*?)\n```", content, re.DOTALL)
generic_blocks = re.findall(r"``\s*\n(.*?)\n``", content, re.DOTALL)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we still the three backticks for these?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. The comment on line 75 () is misleading — the regex actually matches RST-style double-backtick fenced blocks (``), not Markdown triple-backtick blocks. The comment should say double backticks to match the regex. Tracked in #1172 alongside the other RST cleanup work.

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

LGTM assuming that the docs still build and we sort out the other question about triple backticks.

Even though the docs pipeline strips these, we should still try to be consistent in our documentation. I would vote for opening a low priority issue to fix the remaining docstring formatting issues.

Will approve so that it can be merged when ready and prevent as many conflicts as possible.

planetf1 added 2 commits May 27, 2026 15:03
Replace RST-style ``symbol`` notation with Markdown-style `symbol`
notation in Python string literals under mellea/ and cli/.  Keeps
fenced code blocks (triple backticks) intact.

Fresh branch off main, superseding PR generative-computing#659.  Fixes generative-computing#657.

Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Pre-existing RST typo: ``return_sampling_results`` was missing its
opening double-backtick, leaving a dangling backtick after the
bulk normalisation.  Correct the full inline-code span.

Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1

Copy link
Copy Markdown
Contributor Author

Tracked as #1172 — assigned to @planetf1, milestone Release 0.6.0, in the current iteration. The regex/comment inconsistency in python_reqs.py is captured there too.

Merged via the queue into generative-computing:main with commit 14704e8 May 27, 2026
9 checks passed
@planetf1
planetf1 deleted the fix/rst-backticks-657 branch May 27, 2026 15:07
akihikokuroda pushed a commit to akihikokuroda/mellea that referenced this pull request May 27, 2026
…#1155)

* docs(api): normalize source docstring backticks

Replace RST-style ``symbol`` notation with Markdown-style `symbol`
notation in Python string literals under mellea/ and cli/.  Keeps
fenced code blocks (triple backticks) intact.

Fresh branch off main, superseding PR generative-computing#659.  Fixes generative-computing#657.

Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

* docs(api): fix malformed docstring in session.py

Pre-existing RST typo: ``return_sampling_results`` was missing its
opening double-backtick, leaving a dangling backtick after the
bulk normalisation.  Correct the full inline-code span.

Assisted-by: Claude Code
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>

---------

Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
yelkurdi added a commit to yelkurdi/mellea that referenced this pull request Jun 1, 2026
Apply the same RST->Markdown backtick normalization upstream did in generative-computing#1155
(``symbol`` -> `symbol`) to the new mellea/stdlib/context/ package files
that were authored on this branch in the old style. Mechanical sweep,
no semantic changes.

Files:
- mellea/stdlib/context/__init__.py    (4 lines)
- mellea/stdlib/context/chat.py        (36 lines)
- mellea/stdlib/context/simple.py      (4 lines)
- mellea/stdlib/context/compactor.py   (200 lines)

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>
psschwei added a commit to nrfulton/mellea that referenced this pull request Jun 8, 2026
Resolves conflicts in mellea/backends/kv_block_helpers.py (taking the
v5 rewrite — main's side was docstring updates from generative-computing#1155 for legacy
functions removed in this PR) and regenerates uv.lock with
transformers==5.10.2.

Updates _HF_INTERNAL_TEMPLATE_VARS for transformers v5: adds
"documents" (now injected into the Jinja namespace by
render_jinja_template alongside messages, tools, add_generation_prompt).
Comment cross-references updated to point at chat_template_utils where
the namespace is actually constructed. test_huggingface_filter_options
expected set updated to match.

Assisted-by: Claude Code
Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>
yelkurdi added a commit to yelkurdi/mellea that referenced this pull request Jun 23, 2026
Apply the same RST->Markdown backtick normalization upstream did in generative-computing#1155
(``symbol`` -> `symbol`) to the new mellea/stdlib/context/ package files
that were authored on this branch in the old style. Mechanical sweep,
no semantic changes.

Files:
- mellea/stdlib/context/__init__.py    (4 lines)
- mellea/stdlib/context/chat.py        (36 lines)
- mellea/stdlib/context/simple.py      (4 lines)
- mellea/stdlib/context/compactor.py   (200 lines)

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>
AngeloDanducci pushed a commit to AngeloDanducci/mellea that referenced this pull request Jul 2, 2026
…mputing#996)

* feat: add context compaction strategies for react framework

Adds CompactionStrategy abstraction and KeepLastN implementation to
mellea/stdlib/compaction.py, wires an optional compaction parameter into
the react() loop, and adds full test coverage in test/stdlib/test_compaction.py.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* refactor: express compaction threshold as token count

Switches `CompactionStrategy.threshold` from a component-count trigger to a
token-count trigger, read from the most recent `ModelOutputThunk.usage`
populated by the backend. This aligns compaction with the real constraint
(context size) and sidesteps per-backend tokenizer dependencies by using
provider-reported usage; the trade-off is a one-turn lag since usage is
recorded at the end of each model call.

Also reorders the react loop so compaction runs after the final-answer
check, skipping wasted work (and a wasted LLM call for LLMSummarize) on
terminal turns.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* Fix mot.generation.usage

Signed-off-by: ramon-astudillo <ramon@astudillo.com>

* refactor: relocate compaction module into frameworks package

Move the compaction strategies alongside the react framework they serve:
- mellea/stdlib/compaction.py -> mellea/stdlib/frameworks/react_compaction.py
- test/stdlib/test_compaction.py -> test/stdlib/frameworks/test_react_compaction.py

Imports and module docstrings updated accordingly.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs: add Args/Returns sections to react_compaction compact overrides

The docstring quality gate (tooling/docs-autogen/audit_coverage.py
--quality --threshold 100) requires each documented symbol to have its
own Args/Returns sections — inheritance from the abstract parent is not
consulted. Six issues were reported against the compact() overrides on
ClearAll, KeepLastN, and LLMSummarize.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* feat(compaction): per-turn Compactor protocol for ChatContext + ReACT

Replaces the original async ``react_compaction`` strategies (ClearAll,
KeepLastN, LLMSummarize) with a generic, sync ``Compactor`` protocol
that operates on any ``Context``. ``ReACT`` and ``ChatContext`` are
rewired around the new protocol; sample callers, tests, and docs are
updated.

Squash of 29 Mellea-side commits from
context_compaction_for_react_2; the BCP eval harness commits in that
branch are intentionally excluded.

mellea/stdlib/context/ becomes a package
  - Compactor protocol: sync ``compact(ctx, *, backend=None) -> Context``
  - WindowCompactor(size, pin_predicate)
      keep last-N body components; ``size=0`` clears the body and
      retains only the pinned prefix
  - ThresholdCompactor(inner, threshold)
      token-gated wrapper that reads cumulative context size from the
      most recent ModelOutputThunk's ``generation.usage`` and forwards
      to ``inner.compact`` only above the gate
  - LLMSummarizeCompactor(keep_n, pin_predicate, prompt_template)
      summarizes old body components via the backend; the (async)
      backend call is hidden behind a sync ``compact()`` via
      ``_run_coro_blocking`` so the protocol stays sync
  - PinPredicate API: ``pin_nothing``, ``pin_system``,
    ``pin_system_and_initial_user``; chat compactors compose freely

mellea/stdlib/frameworks/react.py
  - ``react()`` gains a ``compactor: Compactor | None = None`` per-turn
    hook; invoked once after each tool observation
  - The old ``react_compaction`` module is removed

mellea/stdlib/components/react.py
  - ``pin_react_initiator``: a PinPredicate that pins everything up to
    and including the first ``ReactInitiator``
  - ``react_summary_prompt(goal=None, max_tokens_hint=None)``: factory
    that returns a research-flavoured summary prompt template (with the
    {conversation} placeholder LLMSummarizeCompactor expects). Optional
    ``GOAL: <goal>`` line and optional ``- Be at most ~N tokens`` bullet
    when callers want goal anchoring or length-cap hints.

mellea/stdlib/context/chat.py
  - ``ChatContext()`` defaults to no compactor (full history); pass
    ``compactor=`` or ``window_size=`` for opt-in compaction. Matches
    upstream main's window_size=None unbounded semantics.

Test coverage
  - test/stdlib/test_compactor.py (~500 LOC): protocol semantics;
    Window / Threshold / LLMSummarize behaviours; pin-predicate edge
    cases; ``size=0`` collapse; threshold gate edge cases
  - test/stdlib/frameworks/test_react_framework.py (~210 LOC):
    react() per-turn hook integration + react_summary_prompt
    (default, goal interpolation, brace escaping, max_tokens_hint
    bullet ordering, LLMSummarizeCompactor template-validation)
  - test/stdlib/test_base_context.py: pin-non-compacting ChatContext
    in the session-copy operations test (matches new opt-in default)

Net diff: 17 files, +381 / -896 lines (drops the old
react_compaction.py and its dedicated test file).

Backwards-compatible default behaviour preserved: bare ``ChatContext()``
retains full history; ``react()`` without ``compactor=`` behaves
identically to today; ``LLMSummarizeCompactor`` defaults to a generic
conversation-summary prompt unless callers opt in to the
research-flavoured variant via ``react_summary_prompt``.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs: fix stale references to ChatContext default and compaction examples

- Correct comment in test_base_context.py: ChatContext defaults to
  compactor=None, not WindowCompactor(5).
- Point compactor.py module docstring to docs/examples/context/ instead
  of the nonexistent docs/rewrite/.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* feat(compaction): InlineCompactor marker + required default_backend on LLMSummarize

Restrict ChatContext(compactor=...) to compactors that inherit InlineCompactor.
Wiring a backend-requiring compactor (e.g. LLMSummarizeCompactor) directly
would invoke the backend on every add(); the new isinstance guard rejects
that with a TypeError pointing at react(compactor=...), ThresholdCompactor,
or manual compact() as alternatives. ThresholdCompactor remains accepted
regardless of its inner -- it gates by token usage, so backend calls are
sparse rather than per-add.

LLMSummarizeCompactor now takes a required default_backend at construction
and falls back to it when compact() is invoked without an explicit backend.
A backend kwarg passed to compact() still overrides the default for that
call. This makes ThresholdCompactor(LLMSummarizeCompactor(...)) work
end-to-end when attached to ChatContext: at trip time, the inner uses its
stored default_backend.

InlineCompactor carries the compact() signature (raising NotImplementedError)
so it's a usable static type without cast() workarounds. Specialized to
ChatContext rather than parameterized over Context -- ThresholdCompactor's
prior generic-T signature was unexercised, so the simpler shape applies.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* fix(session): track interaction_count out-of-band; doc compaction semantic shift

Reviewer flagged that ChatContext(window_size=N) used to keep full history
in as_list() and only window view_for_generation(); the per-turn compactor
work made as_list() itself reflect the post-compaction state, which silently
undercounted in MelleaSession.cleanup() (interaction_count = len(as_list())).

- ChatContext docstring: add a Note describing the semantic shift and
  pointing callers at out-of-band turn tracking when full counts matter.
- MelleaSession: turn ctx into a property; the setter increments
  _interaction_count, and reset() / __init__ bypass via _ctx so lifecycle
  events don't pollute the count. cleanup() now publishes
  self._interaction_count, stable under any compaction strategy.
- SessionCleanupPayload: rewrite the interaction_count field doc to
  match the new semantics ("turns committed" rather than "items in
  context").

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* fix(compaction): make LLMSummarizeCompactor backend errors non-fatal

Reviewer flagged that an exception from the summarisation backend call
(rate limit, network error, timeout) propagates through _run_coro_blocking
and kills the entire react loop. For long-running research tasks that's
quite painful, especially since compaction is best-effort by nature.

LLMSummarizeCompactor.compact() now wraps _run_coro_blocking in a
try/except Exception. On failure it logs a WARNING via MelleaLogger with
the exception type and message, then returns ctx unchanged. The next
compact() invocation retries; the conversation keeps growing in the
meantime. BaseException (KeyboardInterrupt, SystemExit) still propagates
so users can interrupt a stuck loop.

Added a regression test with a backend that raises RuntimeError on every
call: compact() returns the same ctx, original history is intact, and a
warning naming the exception type is logged.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs: stronger warning on _run_coro_blocking event-loop blocking

Reviewer flagged that _run_coro_blocking, when invoked from inside an async
caller like react(), blocks the entire event loop for the full duration of
the wrapped coroutine — not just the calling task. The previous "fine for a
serial ReACT loop" wording undersold the implications.

Beefed-up Warning: block now spells out:
- The loop, not just the thread, is stalled — callbacks, telemetry,
  cancellation signals, other sessions sharing the loop, keepalives are
  all blocked.
- Backends with per-loop resources (notably httpx.AsyncClient) may behave
  unexpectedly because the coroutine runs on a fresh loop in a worker
  thread; documents the typical failure signatures.
- Long-term direction is an async variant on the Compactor protocol so
  callers can await natively.

Docs-only; no behavior change.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* fix(compaction): make LLMSummarizeCompactor rendering loss-aware

Reviewer flagged silent drops when rendering the slice fed to the
summariser:

- Message: append "[N image(s) attached]" / "[M document(s) attached]"
  markers; bytes not reproduced.
- ModelOutputThunk: render "assistant called tools: name({args}), ..."
  for tool-call-only thunks (value=None) and "assistant: <empty>" for
  empty thunks. Eliminates "assistant: None".
- Catch-all else: "<TypeName: content>" or "<TypeName>" instead of the
  default object repr (e.g. ReactInitiator when not pinned).

Docstring gains a Note: that summaries are text-only and lossy for
multimodal / heavy-tool sessions. Tests cover each branch.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* refactor(compaction): skip empty ModelOutputThunks instead of rendering "<empty>"

A "<empty>" marker for thunks with neither value nor tool_calls tended to
leak into the resulting summary verbatim. These turns carry no information
worth summarising, so drop them from the rendered slice entirely. Test
updated to assert no line is emitted for an empty thunk while neighbouring
turns still come through.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* chore(compaction): set silence_context_type_warning on internal aact call

Reviewer flagged that aact's context-type warning could be noisy under
ThresholdCompactor-driven repeated compaction. Match react.py's pattern of
setting silence_context_type_warning=True on internal framework calls so
the warning stays quiet if the context argument is later changed to a
non-SimpleContext, and to self-document the intent.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* feat(compaction): expose model_options on LLMSummarizeCompactor

Reviewer flagged that the summary call uses the backend's default
max_tokens (often 256-512 on local backends), silently truncating long
summaries. react_summary_prompt(max_tokens_hint=N) is only a soft prompt-
side nudge, not a real API parameter.

Add model_options: dict | None to the constructor and forward it to
mfuncs.aact so callers can set a hard token budget (or any other backend
option). Default None preserves existing behaviour. Tests cover both
forwarded and default paths.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* chore(docs): normalize source docstring backticks in context package

Apply the same RST->Markdown backtick normalization upstream did in generative-computing#1155
(``symbol`` -> `symbol`) to the new mellea/stdlib/context/ package files
that were authored on this branch in the old style. Mechanical sweep,
no semantic changes.

Files:
- mellea/stdlib/context/__init__.py    (4 lines)
- mellea/stdlib/context/chat.py        (36 lines)
- mellea/stdlib/context/simple.py      (4 lines)
- mellea/stdlib/context/compactor.py   (200 lines)

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* fix(docs): make custom_compactor.py example actually run

TruncateOldest didn't inherit InlineCompactor, so the Pattern 1
demonstration crashed with TypeError when ChatContext's __init__
guard rejected it. The example pre-dated the InlineCompactor marker
and was never updated.

Inherit InlineCompactor on TruncateOldest, rewrite the module
docstring to describe the two-pattern setup honestly (mentioning
ThresholdCompactor as the wrap for backend-calling compactors), and
move the "pure structural typing" demonstration to a fresh inline
JustCompact class so it still proves the bare Compactor protocol
works without inheritance. Also normalises backticks in this file
(missed by the package-only sweep in 298ee27).

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* fix(compaction): re-raise programming errors instead of masking as backend failure

The previous broad `except Exception` in LLMSummarizeCompactor.compact()
caught both transient backend failures (the design intent) and programming
errors like TypeError/AttributeError/AssertionError/LookupError. Genuine
contract violations would be swallowed as a "warning logged, ctx unchanged"
outcome and could go undetected for a long time.

Add a narrow re-raise for the programming-error classes before the broad
catch — the "best-effort retry" promise still holds for transient backend
failures (RuntimeError, network, timeout, etc.), but real bugs surface
immediately. LookupError covers KeyError and IndexError. Note section in
the docstring is updated accordingly.

Two regression tests added:
- TypeError raised by the backend propagates out of compact() (existing
  RuntimeError test still passes — RuntimeError isn't in the re-raise set).
- KeyboardInterrupt and other BaseException subclasses propagate
  unchanged across the _run_coro_blocking thread bridge — guards the
  docstring contract against future widening to `except BaseException`.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs(context): make ChatContext compaction docstring age-proof

Drop "now" from the compaction notes in ChatContext — without an anchor
it loses meaning over time. Also remove the "Earlier versions kept..."
release-note framing; the undercount warning stands on its own.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs(context): drop caller list from _rebuild_chat_context docstring

Reviewer pointed out that listing specific callers (WindowCompactor)
rots — and indeed LLMSummarizeCompactor now uses it too. Drop the
caller line; keep just what the function does and why (sidestep
ChatContext.add to avoid compactor recursion).

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* refactor(compactor): hoist top-level imports out of function bodies

Reviewer flagged the inline Message import in pin_system. Hoisted the
imports that have no circular-dependency risk (Message, ToolMessage,
MelleaLogger, _rebuild_chat_context, SimpleContext) to the top of the
module. Left mfuncs lazy and added a comment explaining why: it imports
SimpleContext via the context package init, which re-exports from this
module — hoisting it would hit a partial-package ImportError.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* refactor(compactor): avoid wasted slice in LLMSummarizeCompactor.compact

The body slice was only consumed by len() before being discarded
(_async_compact recomputes its own body). Replace with body_len
arithmetic, matching the existing pattern in WindowCompactor.compact.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* refactor(compactor): pass precomputed full/pin_end into _async_compact

LLMSummarizeCompactor.compact() and _async_compact were independently
calling ctx.as_list() and self.pin_predicate(full). Thread the values
through instead so the async core skips the duplicate work.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* refactor(compactor): pythonic enumerate loop in pin_system

Replace the while-with-index pattern in pin_system() with a cleaner
enumerate-based early-return loop. Behavior unchanged.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs(react): note that compactor must return a ChatContext

The Compactor protocol is generic in T bound to Context, so a custom
compactor returning a non-chat Context (e.g. SimpleContext) type-checks
at the react() call site but breaks the ChatContext assumptions the
next aact call relies on. Document the constraint on the compactor=
parameter so callers don't get surprised at runtime.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs(examples): note system-message eviction caveat in TruncateOldest

Reviewer pointed out that items[1:] drops index 0 unconditionally, so
a leading system message gets evicted on first compaction. Add a Note
to the class docstring flagging the caveat and pointing readers at
WindowCompactor + pin_system for the keep-the-system-prompt variant.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs(compactor): fix misleading "T = ChatContext" docstring framing

Reviewer pointed out there's no class-level T = declaration — T is a
per-call TypeVar on compact(). Reword to describe the override
annotation directly: chat-only compactors override compact() with a
ChatContext signature.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs(compactor): add Raises entry for InlineCompactor.compact

Reviewer flagged the docstring gap: compact() raises NotImplementedError
unconditionally on the marker base class, but the docstring had no
Raises: section. Add one noting the always-raises behavior and that
subclasses are expected to override.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs(compactor): add Args/Returns sections to satisfy doc quality gate

The build-and-validate CI job's --fail-on-quality threshold flagged 8
issues across pin_nothing, pin_system, pin_system_and_initial_user, and
InlineCompactor.compact (missing Args + Returns). Add the sections.
Also collapse a multi-line _run_coro_blocking call back to one line so
ruff format passes.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* test: override _generate_from_raw in compactor test fakes

The Backend ABC's abstract method was renamed to _generate_from_raw
(leading underscore) in generative-computing#1264; the public generate_from_raw is now a
concrete hook-firing wrapper. The fakes in test_compactor.py overrode
the wrapper but left the abstract method unimplemented, so each backend
raised TypeError on instantiation.

Rename generate_from_raw -> _generate_from_raw in FakeBackend,
OtherBackend, BrokenBackend, BuggyBackend, and InterruptingBackend.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* test: assert compactor returns ChatContext in react loop

Mirror the guard on the aact next_context returns: the LLMSummarizeCompactor
docstring notes a non-ChatContext return would break the loop, but nothing
enforced it. Add an isinstance assert after compact() to fail fast.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* fix(compaction): widen compactor types for upstream MOT as_list change

as_list() now returns list[Component | CBlock | ModelOutputThunk]; align
PinPredicate, pin_* helpers, and _async_compact. Update the
react_compaction example backend to the new Backend API (widened
_generate_from_context, override _generate_from_raw not the final
generate_from_raw).

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

* docs(compaction): document window_size pin_system semantic change

window_size=N maps to WindowCompactor (default pin_system), a deliberate
change from the old raw last-N as_list(N) view: the system message is now
pinned and size counts only the non-pinned body. Note the old drop-in
(pin_nothing) in the docstring and add a CHANGELOG behavior-change entry
covering both this and the as_list()-now-windowed move.

Assisted-by: Claude Code
Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>

---------

Signed-off-by: Yousef El-Kurdi <yelkurdi@gmail.com>
Signed-off-by: ramon-astudillo <ramon@astudillo.com>
Co-authored-by: ramon-astudillo <ramon@astudillo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: replace RST double-backtick notation with single backticks in source docstrings

3 participants