-
Notifications
You must be signed in to change notification settings - Fork 0
Add examples, mermaid diagram and exit step #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
8dcb2d7
to
730e27b
Compare
lib/mars/agent.rb
Outdated
chat.ask(input) | ||
end | ||
|
||
def to_mermaid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now I'm ok, but I'd like to have any diagraming code completely decoupled from the main code. Something like Mars::UI::Renderer.new(workflow).to_mermaid
. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean decoupled in a way that it could actually live on its own in a different gem, but Mars don't really need to care about the rendering part (now is mermaid, but we could have multiple different renderers)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I liked this approach because we could use polymorphism to keep it easy to extend to new objects (each component only needs to know how to render itself), so adding a new component requires just implementing the interface.
That being said, I agree that this is a separate concern and could live in a different layer, possibly different gem, since it's not the core logic and so the objects shouldn't be coupled to it. Let me think of something more aligned to this separation while still keeping the extensibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good example (see how visualize_packs
is separate from packs
)
I'm not saying to have separate gems (it's not worth it for now) but if we can have clear boundaries from the beginning then that could be helpful.
Regarding the solution maybe we can still benefit from this polymorphic solution without having to couple things, for example doing something like:
module Mars
class Agent
# core logic
end
end
module Mars
module UI
module Agent
def to_mermaid
"#{sanitized_name}[\"#{name}\"]"
end
end
end
end
# Dynamically inject in `lib/mars.rb`
Mars::Agent.include(Mars::UI::Agent)
In the future we may also allow to configure turning on/off the UI module, or changing it for a different UI module for example?
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, I will try something like that.
lib/mars/agent.rb
Outdated
def can_end_workflow? | ||
true | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm why only agents can end workflows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The start workflow can end on a gate for example, here it branches out to another component but doesn't end the main flow. However, it ends if the last node is an agent instead. Later on, I think aggregators could also indicate that the flow ended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure I'm following. I don't really see the value of this method
No description provided.