fix: python decompose model change and pipeline fix#569
Conversation
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
|
@psschwei that issue you ran into should be solved with this update @csbobby could use your feedback as well, the change in It looks like this area is getting reworked in #556 so maybe this is fine in the interim. |
| "validation_strategy": constraint_validation_strategies.get( | ||
| cons_str, "llm" | ||
| ), |
There was a problem hiding this comment.
@csbobby, can you please confirm that you are okay with changing this and always having an LLM default?
There was a problem hiding this comment.
Hi @AngeloDanducci @jakelorocco Thank you for sharing this.
Yes, adding a fallback setup "llm" looks good point.
- We tested the extraction feature by using the mainstream models/APIs. Typically, it works well on both regular or larger sizes (7B or +). We don't recommend to use the weaker llms since the output quality will be potentially different or unpredictable. But if any user encounter it, the "llm" seems will helpful to provide a best try on meaningful constraint content instead of a program error. If you think this satisfy our intent Mellea usage for users, also looks good to me.
About the fix, the ideal position I think is the first position of the error occurred.
- The cons_key is the validation strategy, and the cons_str is the validation type (llm/code) for the following up execution. The extraction failure (if there is) of the validation type will be detected on the validation_decision.generate(m_session, cons_key).parse() at (L119 ).
- I would suggest to add the fallback to the extraction failure position at (L119 ) rather than other following steps (e.g., on the returned DecompPipelineResult (L145-147) in the original fix).
That would keep the consistency for the fallbacked cons_str.
Line 119
constraint_validation_strategies: dict[str, Literal["code", "llm"]] = {
cons_key: validation_decision.generate(m_session, cons_key).parse() or "llm"
for cons_key in task_prompt_constraints
}
There was a problem hiding this comment.
Thanks @csbobby - I've added the default value to line 119.
However I've also kept the update on 145.
I think the update on line 145 handles cases where a constraint in subtask_data.constraints doesn't exist as a key in constraint_validation_strategies at all (which happened when I ran this example).
15d8fff
Misc PR
Type of PR
Description
Testing