align exclude_events default with hooks-logging: drop llm:stream_block_delta by default#28
Merged
Merged
Conversation
…k_delta by default
Both hook-context-intelligence and amplifier-module-hooks-logging now default to
excluding ["llm:stream_block_delta"] from their respective sinks — the local
events.jsonl AND the graph-server dispatch — without sharing any code.
Root cause of the divergence:
hook-context-intelligence's ConfigResolver.exclude_events defaulted to []
(empty frozenset), so the per-token streaming delta flooded both the JSONL
audit log and graph dispatch unless users manually set exclude_events.
amplifier-module-hooks-logging already used _DEFAULT_EXCLUDE_EVENTS =
["llm:stream_block_delta"] on its feat/exclude-events branch.
What changed:
- config_resolver.py: added _DEFAULT_EXCLUDE_EVENTS: list[str] =
["llm:stream_block_delta"] module-level constant with a comment explaining
the intentional value-alignment with hooks-logging (not shared code).
- ConfigResolver.exclude_events property: updated default from [] to
_DEFAULT_EXCLUDE_EVENTS; updated docstring.
- __init__.py module docstring: updated exclude_events config-key description
to document the default and the opt-out pattern.
- tests/test_config_resolver.py: updated test_defaults_to_empty_set
(now test_defaults_to_block_delta) and added 6 new tests:
explicit [] disables filter, block_delta excluded, block_start/end/aborted
and llm:response NOT excluded by default.
Semantics preserved:
exclude_events: [] → frozenset() → filter disabled (log/dispatch everything)
exclude_events unset → frozenset({"llm:stream_block_delta"}) → delta suppressed
The existing 'if exclude else events' registration gate in __init__.py already
ensures excluded events are never registered → suppressed from BOTH local write
and graph dispatch.
No shared module/import between the two hooks — alignment is by identical value
only, per the deliberate decoupling requirement.
Backwards-compatible: llm:stream_block_delta is emitted by the new streaming
providers (feat/exclude-events / streaming-contract work); existing deployments
without streaming providers see no change.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Change _DEFAULT_EXCLUDE_EVENTS from ["llm:stream_block_delta"] to
["llm:stream_*delta"] — a fnmatch glob that expresses the transient-
streaming-delta *category* rather than one hardcoded event name.
The pattern comes from the "Event dispositions" convention in the
provider streaming contract. It is intentionally IDENTICAL to the
default used by amplifier-module-hooks-logging, aligned by that
convention and NOT by shared code. The two hooks must remain
decoupled; keep them in sync via the contract, never by extracting
a shared module.
The glob matches llm:stream_block_delta (the current per-token delta
event) and would absorb any future *delta variants, while sparing
the structural streaming events (block_start, block_end, stream_aborted).
Changes:
- config_resolver.py: constant value + comment + property docstring
- __init__.py: module docstring (configuration keys section)
- test_config_resolver.py: update TestExcludeEvents — default is now
{"llm:stream_*delta"}; delta-excluded test uses fnmatch.fnmatch;
structural-event tests use fnmatch; new
test_glob_spares_structural_streaming_events asserts the full
match/no-match matrix explicitly
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Auto-reformatted by `ruff format .` to bring two files into compliance with the repo's 100-char line-length limit (pyproject.toml). - config_resolver.py: wrap the frozenset(...) call in exclude_events that exceeded 100 chars - tests/test_config_resolver.py: wrap two long test-method docstrings No logic changes. Both CI commands now pass locally: uv run ruff format --check . → 84 files already formatted uv run ruff check . → All checks passed! 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Aligns the hook-context-intelligence
exclude_eventsdefault with amplifier-module-hooks-logging — both now default to['llm:stream_block_delta'], keeping the high-frequency per-token streaming delta out of the local events.jsonl AND the graph-server dispatch by default (opt back in withexclude_events: []). The two hooks share no code; they are aligned by value and by the common streaming contract. Backwards-compatible: the excluded event ships with the new streaming providers, so existing logs/dispatch are unchanged. This removes the need for users to set exclude_events manually.What
_DEFAULT_EXCLUDE_EVENTS: list[str] = ["llm:stream_block_delta"]module-level constant toconfig_resolver.pywith a comment explaining the intentional value-alignment with hooks-logging (not shared code — deliberately decoupled).ConfigResolver.exclude_eventsproperty to use_DEFAULT_EXCLUDE_EVENTSas the default (previously[]).tests/test_config_resolver.py: renamedtest_defaults_to_empty_set→test_defaults_to_block_deltaand added 6 new tests covering the complete semantics.Why
Before this change, hook-context-intelligence defaulted to
exclude_events: [], meaningllm:stream_block_delta(the per-token streaming event, firing hundreds-to-thousands of times per turn) would flood both the localevents.jsonland the graph-server dispatch unless users manually configured the exclusion.amplifier-module-hooks-loggingalready excluded it by default on itsfeat/exclude-eventsbranch. This change brings the two hooks into alignment.Semantics
exclude_eventsunsetexclude_events: []exclude_events: ["llm:stream_*"]The existing
if exclude else eventsgate in__init__.pyensures excluded events are never registered → suppressed from BOTH the local write and graph dispatch. No logic changes needed there.No shared code
The two hooks are deliberately decoupled.
_DEFAULT_EXCLUDE_EVENTSinconfig_resolver.pyis an independent constant — no import, no shared module, no cross-repo dependency. If the value needs to change in future, both hooks are updated independently.Tests