✨ [WIP] Agents now take parent task history into account #491
Conversation
fabianvf
commented
Nov 14, 2024
fabianvf
commented
- The AnalyzerAgent no longer uses hardcoded language, source, and target, but instead gathers it from the task context
- The AnalyzerAgent no longer uses hardcoded language, source, and target, but instead gathers it from the task context Signed-off-by: Fabian von Feilitzsch <[email protected]>
| if self.parent is not None: | ||
| return self.oldest_ancestor().background() | ||
| if self.children: | ||
| return f"""You are a software developer who specializes in migrating from {" and ".join(self.sources)} to {" and ".join(self.targets)} |
There was a problem hiding this comment.
I don't like having prompt creation on this object, but OTOH these objects know exactly what fields they have. Curious to hear people's thoughts
There was a problem hiding this comment.
I don't see a problem with using this object tbh, feels natural...just rename it to as_llm_context() or as_natural_language() or something
There was a problem hiding this comment.
Yeah maybe I'll just take out the instructions part of it and leave it as a summary, I think that would make it a lot cleaner
| You are an experienced java developer, who specializes in migrating code to the Quarkus Framework | ||
| system_message_template = Template( | ||
| """ | ||
| You are an experienced {{ language }} developer, who specializes in migrating code from {{ source }} to {{ target }} |
There was a problem hiding this comment.
Currently I'm getting really awesome high quality prompts that look like this:
You are an experienced java developer, who specializes in migrating code from to eap8 and eap and jws6+ and hibernate6+ and hibernate and eap8+ and quarkus3+ and jakarta-ee and jws and jakarta-ee9+ and quarkus
You are a software developer who specializes in migrating from to eap8 and eap and jws6+ and hibernate6+ and hibernate and eap8+ and quarkus3+ and jakarta-ee and jws and jakarta-ee9+ and quarkus
You attempted to solve an issue in a repository you are migrating:
Location: file:///example/coolstore/src/main/java/com/redhat/coolstore/model/Order.java
Message:
The way in which Hibernate determines implicit names for sequences and tables associated with identifier generation has changed in 6.0 which may affect migrating applications.
As of 6.0, Hibernate by default creates a sequence per entity hierarchy instead of a single sequence hibernate_sequence.
Due to this change, users that previously used `@GeneratedValue(strategy = GenerationStrategy.AUTO)` or simply `@GeneratedValue` (since `AUTO` is the default), need to ensure that the database now contains sequences for every entity, named `<entity name>_seq`. For an entity Person, a sequence person_seq is expected to exist.
It’s best to run hbm2ddl (e.g. by temporarily setting `hbm2ddl.auto=create`) to obtain a list of DDL statements for the sequences.
However your solution caused additional problems elsewhere in the repository, which you are now going to solve.
and
You are an experienced xml developer, who specializes in migrating code from java-ee and java to cloud-readiness
None
There was a problem hiding this comment.
So refinement is needed to say the least
There was a problem hiding this comment.
Also we potentially may want to add the TaskResult objects to the Tasks, which would allow us to include some specific information about the changes made prior in the prompt as well (perhaps it would also make sense to generate and attach the diff in the TaskResult?)
| @@ -22,30 +22,27 @@ | |||
| @dataclass | |||
| class _llm_response: | |||
| reasoning: str | None | |||
| java_file: str | None | |||
| source_file: str | None | |||
There was a problem hiding this comment.
unrelated, can we make this a Path, I see str and Path both in the codebase
| @@ -117,64 +114,79 @@ def execute(self, ask: AgentRequest) -> AnalyzerFixResponse: | |||
|
|
|||
| language = guess_language(ask.file_content, file_name) | |||
|
|
|||
| source = " and ".join(ask.sources) | |||
There was a problem hiding this comment.
I think its OK to put only one of the sources, it is not going to hurt the result a lot
There was a problem hiding this comment.
it's just sometimes the source is just java which I feel like could be misleading
There was a problem hiding this comment.
looks like we have got to add a new label on rules that we can use as a hint here, wdyt?
There was a problem hiding this comment.
Yeah maybe we can add a special label and if it's present grab that instead. Not sure how hard it would be up add but probably not that bad
| if self.parent is not None: | ||
| return self.oldest_ancestor().background() | ||
| if self.children: | ||
| return f"""You are a software developer who specializes in migrating from {" and ".join(self.sources)} to {" and ".join(self.targets)} |
There was a problem hiding this comment.
I don't see a problem with using this object tbh, feels natural...just rename it to as_llm_context() or as_natural_language() or something
