Add ControlFlowTask
#913
Add ControlFlowTask
#913
Conversation
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
vachillo
force-pushed
the
control_flow
branch
3 times, most recently
from
066e451
to
40b096a
Compare
| @@ -10,11 +10,13 @@ class BooleanArtifact(BaseArtifact): | |||
| meta: dict[str, Any] = field(factory=dict, kw_only=True, metadata={"serializable": True}) | |||
|
|
|||
| @classmethod | |||
| def parse_bool(cls, value: Union[str, bool]) -> BooleanArtifact: | |||
| def parse_bool(cls, value: Union[str, bool, TextArtifact]) -> BooleanArtifact: | |||
There was a problem hiding this comment.
Should be named from_bool for consistency with other similar methods.
There was a problem hiding this comment.
- a bool is one of the possible inputs, so
from_boolisn't really accurate. - i wanted to convey the fact that its going to actually evaluate the literal text in the string, rather than just coerce the input type to a bool type.
| @@ -10,11 +10,13 @@ class BooleanArtifact(BaseArtifact): | |||
| meta: dict[str, Any] = field(factory=dict, kw_only=True, metadata={"serializable": True}) | |||
|
|
|||
| @classmethod | |||
| def parse_bool(cls, value: Union[str, bool]) -> BooleanArtifact: | |||
| def parse_bool(cls, value: Union[str, bool, TextArtifact]) -> BooleanArtifact: | |||
| """ | |||
| Convert a string literal or bool to a BooleanArtifact. The string must be either "true" or "false" with any casing. | |||
There was a problem hiding this comment.
Update docstring to mention text artifact.
| self.structure.meta_memory.add_entry( | ||
| ControlFlowMetaEntry( | ||
| input_tasks=[parent.id for parent in self.parents], | ||
| output_tasks=[child.id for child in filter(lambda child: not child.is_finished(), self.children)], | ||
| output=self.output, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Why are we adding to Meta Memory at all? What does the LLM need to know about the branch decisions?
Also, there has been talk of removing Meta Memory entirely, I'm not sure we should rely on it.
There was a problem hiding this comment.
wanted to have some mechanism of storing the "decision" of the control flow.
griptape/artifacts/task_artifact.py
Outdated
|
|
||
|
|
||
| @define | ||
| class TaskArtifact(ControlFlowArtifact): |
There was a problem hiding this comment.
Not sure how I feel about this artifact. And in a fully declarative Workflow it may be impossible to return a Task object.
Can we just operate with Task ids in a TextArtifact?
There was a problem hiding this comment.
this is just used to return the input of the task. and it will always return the tasks because it pulls directly from the structure
| from griptape.tasks import BaseTask | ||
|
|
||
|
|
||
| class BooleanControlFlowTask(BaseControlFlowTask): |
There was a problem hiding this comment.
I'm not sure we need to reinvent boolean logic in its own Task. Users can implement this themselves in their control_flow_fn
|
|
||
|
|
||
| @define | ||
| class ChoiceControlFlowTask(BaseControlFlowTask): |
There was a problem hiding this comment.
Do we need a dedicated Task for this? What if we added control_flow_fn to BaseTask?
There was a problem hiding this comment.
future proof if there is other types of control flow tasks
|
Slight variation to this implementation: dev...feature/branch-task |
ControlFlowTask,ControlFlowArtifact,TaskArtifactCANCELLEDCloses Add support for branching in
Workflows #904