Conversation
44479d5 to
f06e862
Compare
luyuzhe111
left a comment
There was a problem hiding this comment.
@Linbo-Liu you might need to install the pre-commit hooks properly. a lot of the cosmetic wraps seem to stem from using the default 88-char ruff line limit instead of the 120-char limit specified in this repo.
3c32052 to
fd16a25
Compare
| "3. Update all dependencies to their Java 17 compatible versions" | ||
| ) | ||
| tools.append(search_dependency_version) | ||
| elif agent_type == "hybrid": |
There was a problem hiding this comment.
wondering what's the interaction between agent type and require_maximal_migration? The hybrid type calls update_jdk_related() + update_dependency_version() (which presumably does maximal migration preprocessing), but require_maximal_migration is a separate boolean. can we make it more explicit?
There was a problem hiding this comment.
based on my understanding, the hybrid agent type isn't really a different agent — it's a preprocessing toggle (apply static dependency updates before the agent runs). Making this an explicit boolean like apply_static_update would be clearer and composable with other flags like require_maximal_migration. Same argument could apply to rag — these feel like independent feature flags, not mutually exclusive agent types. wondering what's your thought?
There was a problem hiding this comment.
You are right. If one uses hybrid agent, the implicit intention is to do max migration. However, I didn't set this explicit because technically speaking, if doing minimal migration, hybrid agent can downgrade the dependency versions even if they were already upgraded by static script...
There was a problem hiding this comment.
hybrid agent type isn't really a different agent
It uses a different prompt. RAG agent is the same: different prompt. Maybe I should change agent_type to prompt_type?
There was a problem hiding this comment.
@Linbo-Liu thanks for the explanation! it makes a lot of sense.
in this case, I would suggest flattening it into two independent boolean flags: apply_static_update, and use_dependency_search_tool. It will justify the following changes better than a single prompt type.
- for
setup_repo_environment, it will take an additional arg,apply_static_update. it's semantically much clearer. withprompt typeas input, people get confused how prompt type is related to the repo setup procedure. - it will also be more intuitive imo to update both the tool and prompt based on the
use_dependency_search_toolflag rather than prompt type.
6e0116e to
512aa58
Compare
512aa58 to
a99055c
Compare
a99055c to
cc617ae
Compare
|
thanks for the great work and the iterations! |
Issue #, if available:
Description of changes:
eval_util.py,evaluate.py, andevaluate_async.py.pyproject.tomlREADME.md.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.