Skip to content

#118 action plan should include multiple files#119

Open
n3o2k7i8ch5 wants to merge 30 commits into
developfrom
118-action-plan-should-include-multiple-files
Open

#118 action plan should include multiple files#119
n3o2k7i8ch5 wants to merge 30 commits into
developfrom
118-action-plan-should-include-multiple-files

Conversation

@n3o2k7i8ch5
Copy link
Copy Markdown
Contributor

@n3o2k7i8ch5 n3o2k7i8ch5 commented May 22, 2025

Closes #118

image

@n3o2k7i8ch5 n3o2k7i8ch5 requested a review from stxpatryk as a code owner May 22, 2025 17:46
…ould-include-multiple-files

# Conflicts:
#	libs/core/deep_next/core/steps/action_plan/action_plan.py
#	libs/core/deep_next/core/steps/gather_project_knowledge/project_description/generate_project_description.py
#	libs/core/deep_next/core/steps/implement/develop_patch.py
…ould-include-multiple-files

# Conflicts:
#	libs/common/deep_next/common/llm.py
#	libs/core/deep_next/core/steps/action_plan/action_plan.py
#	libs/core/deep_next/core/steps/action_plan/srf/file_selection/graph.py
#	libs/core/deep_next/core/steps/gather_project_knowledge/project_description/generate_project_description.py
@n3o2k7i8ch5 n3o2k7i8ch5 self-assigned this Jun 3, 2025
…ould-include-multiple-files

# Conflicts:
#	libs/core/deep_next/core/steps/action_plan/srf/file_selection/utils.py
…ould-include-multiple-files

# Conflicts:
#	libs/core/deep_next/core/graph.py
#	libs/core/deep_next/core/steps/action_plan/action_plan.py
#	libs/core/deep_next/core/steps/implement/apply_patch/apply_patch.py
#	llm-config.yaml
Comment thread libs/core/deep_next/core/steps/action_plan/action_plan.py
Comment thread libs/core/deep_next/core/steps/action_plan/srs/localize_function.py
Comment thread libs/core/deep_next/core/steps/code_review/graph.py
Comment thread libs/core/deep_next/core/steps/action_plan/graph.py
Comment thread libs/common/deep_next/common/llm.py Outdated
Comment on lines +251 to +254
seed (int | None): Optional seed for reproducibility. If provided, it will be
added to the base seed from the config (as long as the config seed is not
None. If the config seed is not provided, this value will be used as seed
itself).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

way too complicated. I'd stick with config based seed and if config seed is something like -1 then we do random. All retry mechanism might just implement +1 mechanism to the config seed. What do you think?

Comment thread libs/core/deep_next/core/graph.py Outdated
issue_comments=issue_comments,
)

def steps_to_str(self, steps: list[Step]):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make it priv

related_code_context = "\n".join(
[f"File: {file_path}\n{read_txt(file_path)}" for file_path in related_files]
[
f"File: {path}\n{read_txt_or_none(path) or '<Failed to read file>'}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that might be an issue with files. From my perspective silent exception is a big problem. What do You think of making it regular loop and do the error handling here? read_txt_or_none is created in core.io only for this one usage, isn't that unnecessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, but what would you like to do with an FileNotFound exception in the loop? Raise a warning, or something else?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log warning if byte object or if file not found

Comment thread libs/core/deep_next/core/steps/action_plan/action_plan.py
Comment thread libs/core/deep_next/core/steps/action_plan/action_plan.py

for step in action_plan.ordered_steps:
for target_file in step.target_files:
abs_path = root_path / target_file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not to keep try_to_resolve_path?

FileCodeContext(
path=relevant_file.path,
code_snippet=read_txt(relevant_file.path),
code_snippet=read_txt(state.root_path / relevant_file.path),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is also a good candidate for try_to_resolve_path

Comment thread libs/common/deep_next/common/llm.py Outdated
Comment on lines +338 to +339
if (seed := config.seed) is None:
seed += seed_increment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it will fail None += seed_increment, shouldn't be not None?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

action plan should include multiple files

3 participants