feat: add debug plugin collections#1251
Conversation
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
ajbozarth
left a comment
There was a problem hiding this comment.
Useful addition. Main asks are inline; one cross-cutting item:
Missing unit tests. AGENTS.md §10 and the PR template require unit tests for new code; the 7 example scripts are ollama/e2e gated and won't run in the fast tier. Please add test/plugins/test_builtin_debug.py with caplog-based assertions covering at least:
- Each hook fires and logs the expected line on a synthetic payload
- The empty/missing-field paths the defensive
getattrcode is reaching for (payload.action = None,model_output.value = None)
Minor: the example block in the PR description has trailing prose inside the code fence — worth fixing so it renders.
planetf1
left a comment
There was a problem hiding this comment.
Two things worth flagging at the PR level:
Missing sidebar entry: docs/docs/how-to/debug-with-plugins.md isn't listed in docs/sidebars.ts, so it'll build fine but won't appear in the site navigation. Needs an entry like 'how-to/debug-with-plugins' added to the how-to array.
No test coverage: The new builtin_debug modules don't have any tests — test/plugins/ would be a natural home. The three helper functions (_get_prompt_preview, _get_response_preview, _get_token_usage) have branchy logic that's easy to unit-test with synthetic payloads, and a quick import smoke test would have caught the GenerationTracingPlugin issue below. Worth a test/plugins/test_builtin_debug.py.
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
ajbozarth
left a comment
There was a problem hiding this comment.
Some feedback from Claude.
Prior review items addressed; tests pass.
plugin_scope was applied to builtin_generation_tracing.py but not the other five
builtin_*.py examples — they still call register() at module scope, which leaks
hooks if anyone copies them into a notebook or long-running app. Worth converting all
five to match the generation example, e.g.:
# at top
from mellea.plugins import plugin_scope
# in main()
with plugin_scope([log_validation_pre_check, log_validation_post_check]):
with mellea.start_session() as m:
...
Files: builtin_complete_diagnostics.py, builtin_full_pipeline_tracing.py,
builtin_sampling_diagnostics.py, builtin_validation_failures.py,
builtin_validation_strict.py, builtin_validation_tracing.py.
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
Pull Request
Issue
Fix: #1250
Description
description is in issue #1250
Testing
Attribution
Adding a new component, requirement, sampling strategy, or tool?
If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.
NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.