Repository navigation
Conversation
Align instruct_validate_repair/, generative_stubs/, context/, and sessions/ example directories with the standardized 2-column Lean Hub navigation schema. Add comprehensive header docstrings, Usage commands, and PEP 723 metadata across all scripts, and preserve multi-artifact orchestration guides. Assisted-by: IBM Bob Signed-off-by: Rafael Vasquez <rafvasq21@gmail.com>
github-actions
Bot
added
the
documentation
rafvasq
changed the title
ajbozarth
left a comment
ajbozarth
left a comment
There was a problem hiding this comment.
FYI there's an open PR to update and extend the qiskit IVR example #1716 that I'm actively reviewing, and on a quick glance I believe the edits to it in this PR might be overly deletion heavy, but I still need to take a deeper look. I did miss my chance to review your first PR, but it didn't touch any examples I'd worked on before.
Marking as requests changes for now to block merge, and I'll take a detailed look when I have more time
markstur
left a comment
markstur
left a comment
There was a problem hiding this comment.
commented inline where some descriptions are not accurate.
Bigger concern is whether all the deleted stuff is really not useful. Asked AI and the results were pretty good. Most of what is deleted exists in the docs. The one notable gap was compation. Pasted that feedback inline.
| | :--- | :--- | | ||
| | [`101_email.py`](./101_email.py) | Basic email generation using `m.instruct()` without constraints. | | ||
| | [`101_email_with_requirements.py`](./101_email_with_requirements.py) | Constraining output using string-based requirements with automatic validation and repair. | | ||
| | [`101_email_with_validate.py`](./101_email_with_validate.py) | Explicit validation step using `m.validate()` to inspect requirement satisfaction. | |
There was a problem hiding this comment.
the row for 101_email_with_validate.py claims it demonstrates m.validate(), but the script never calls that; it uses RejectionSamplingStrategy with return_sampling_results=True. The script's own new docstring contradicts the README row.
| | [`101_email.py`](./101_email.py) | Basic email generation using `m.instruct()` without constraints. | | ||
| | [`101_email_with_requirements.py`](./101_email_with_requirements.py) | Constraining output using string-based requirements with automatic validation and repair. | | ||
| | [`101_email_with_validate.py`](./101_email_with_validate.py) | Explicit validation step using `m.validate()` to inspect requirement satisfaction. | | ||
| | [`101_email_comparison.py`](./101_email_comparison.py) | Comparing outputs generated with and without validation requirements. | |
There was a problem hiding this comment.
the row for 101_email_comparison.py says "comparing outputs generated with and without validation requirements," but the script uses no requirements at all — it generates two emails under different instructions and compares them via grounding_context.
|
|
||
| Use `pin_react_initiator` (from `mellea.stdlib.components.react`) as the predicate so the goal and tool registration survive compaction. | ||
|
|
||
| ## Concepts Demonstrated |
There was a problem hiding this comment.
Most of the deleted usage/concept material is covered in the docs. This part is the exception.
compaction. The context/ README previously carried the only conceptual explanation of the Compactor protocol (window vs threshold vs custom, pin predicates, per-add vs per-turn wiring with react()). After this PR, that story survives only in the example docstrings and the auto-generated API reference (api/mellea/stdlib/context/compactor.mdx) — no concepts/ or how-to/ page mentions compaction at all. Since the README collapse is the right call per #1746, should we either add a short compaction section to concepts/context-and-sessions.md or open a follow-up issue so the conceptual docs catch up?
There was a problem hiding this comment.
It seems to me like this is a documentation gap and deserves a follow up issue to include compaction somewhere on https://docs.mellea.ai/next/concepts/context-and-sessions, instead of laying it out in these examples.
|
Small follow up as I have been reviewing #1716 I think you should leave the qiskit example alone and drop those changes from the PR, overall this work feels overly deletion prone, with seemingly no reasoning. I may change my mind with more explanation, but I feel this strategy for example cleanup lessens their usability and standalone nature. Again I only have strong feelings on the qiskit example, and am open to explanations and discussion. |
Signed-off-by: Rafael Vasquez <rafvasq21@gmail.com>
Signed-off-by: Rafael Vasquez <rafvasq21@gmail.com>
|
@ajbozarth no problem! I've reverted touching the Qiskit IVR example in this PR. It'd be helpful if you could review of the tracking issue that tries to capture the way I'm parsing through each of these examples to clean them up (at least as a first-pass) in case you have anything to add there too! |
Pull Request
Issue
Towards #1746
Description
File|Description).context/README compactor descriptions, which should be addressed in the docs themselves (follow-up issue to be opened fordocs/docs/concepts/).Usage:command lines.docs/docs/examples/index.mdto ensure all registered categories match.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.