Feature implementation from commits 70dc62d..9e60c32

[codeOwlAI]

PR Summary

Code Quality Improvements and Feature Enhancements

Overview

This PR includes widespread refactoring to improve code quality, adds support for function calling APIs, implements parallel multi-trace experiment generation, and enhances resource management. It also removes deprecated code related to the v3 proposal version and improves Docker container cleanup mechanisms.

Change Types

Type	Description
Enhancement	Added support for function calling in LLM interactions and parallel multi-trace experiment generation
Refactor	Simplified code patterns, improved type annotations, and removed deprecated functionality
Bugfix	Enhanced resource management for Docker containers with proper cleanup in finally blocks
Removal	Eliminated unused imports, deprecated v3 proposal version support, and DMDockerConf/DMDockerEnv classes

---

Affected Modules

Module / File	Change Description
oai/backend/*	Added function calling support with appropriate interfaces
scenarios/data_science/proposal/exp_gen/parallel.py	Added ParallelMultiTraceExpGen class for asynchronous parallel exploration
utils/env.py	Improved Docker container resource management with cleanup_container function
scenarios/data_science/loop.py	Enhanced trace selection behavior and refactored experiment handling
scenarios/data_science/proposal/exp_gen/proposal.py	Consolidated functionality by removing v3 code and adding function calling
app/qlib_rd_loop/*	Changed report handling and simplified workflow

---

Breaking Changes

• Changed function signatures in scenarios/kaggle/kaggle_crawler.py by removing default parameters
• Changed extract_hypothesis_and_exp_from_reports return value from tuple to single return value
• Changed report_limit from optional to fixed value of 10000
• Changed return type annotations for multiple experiment loaders

---

Notes for Reviewers

• The PR implements consistent error handling patterns with try/finally blocks for resource cleanup
• Function calling support is conditionally enabled based on backend capabilities
• Experiment selection logic has been made more robust for cases with multiple leaves
• Docker container cleanup has been significantly improved with the new cleanup_container utility

[codeowlai]

🐛 Correctness Issue

Potential KeyError Exception on Missing Dictionary Key.

The safety check for 'problem' key existence was removed, which could cause runtime KeyError exceptions if any problem_dict is missing this key.

Current Code (Diff):

- problem_formatted_str += f"## {i+1}. {problem_name}\n"
- problem_formatted_str += f"{problem_dict['problem']}\n"
+ if "problem" not in problem_dict:
+     continue
+ problem_formatted_str += f"## {i+1}. {problem_name}\n"
+ problem_formatted_str += f"{problem_dict['problem']}\n"

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	problem_formatted_str += f"## {i+1}. {problem_name}\n"
	problem_formatted_str += f"{problem_dict['problem']}\n"
	if "problem" not in problem_dict:
	continue
	problem_formatted_str += f"## {i+1}. {problem_name}\n"
	problem_formatted_str += f"{problem_dict['problem']}\n"

[codeowlai]

🐛 Correctness Issue

Docker Container Resource Leak.

The removal of container cleanup calls will cause Docker containers to remain running indefinitely, leading to resource exhaustion over time.

Current Code (Diff):

- print(Rule("[bold green]Docker Logs End[/bold green]", style="dark_orange"))
+ container.stop()
+ container.remove()
+ print(Rule("[bold green]Docker Logs End[/bold green]", style="dark_orange"))

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	print(Rule("[bold green]Docker Logs End[/bold green]", style="dark_orange"))
	container.stop()
	container.remove()
	print(Rule("[bold green]Docker Logs End[/bold green]", style="dark_orange"))

[codeowlai]

🐛 Correctness Issue

Undefined variable self.loop_idx causes AttributeError.

The code references self.loop_idx which is not initialized anywhere in the class, causing an AttributeError at runtime.

Current Code (Diff):

-             report_file_path = self.judge_pdf_data_items[self.loop_idx]
+             report_file_path = self.judge_pdf_data_items[self.loop_n - 1]

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	report_file_path = self.judge_pdf_data_items[self.loop_idx]
	report_file_path = self.judge_pdf_data_items[self.loop_n - 1]

[codeowlai]

🐛 Correctness Issue

Undefined variable self.loop_idx in log statement.

The logging statement references self.loop_idx which is not initialized, causing an AttributeError at runtime.

Current Code (Diff):

-             logger.info(f"Processing number {self.loop_idx} report: {report_file_path}")
+             logger.info(f"Processing number {self.loop_n - 1} report: {report_file_path}")

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	logger.info(f"Processing number {self.loop_idx} report: {report_file_path}")
	logger.info(f"Processing number {self.loop_n - 1} report: {report_file_path}")

[codeowlai]

🐛 Correctness Issue

Potential KeyError Exception.

Direct access to model_data["training_hyperparameters"] without checking if this key exists will cause runtime crashes when processing model data dictionaries that don't contain this field.

Current Code (Diff):

-                training_hyperparameters=model_data["training_hyperparameters"],
+                training_hyperparameters=model_data.get("training_hyperparameters", {}),

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	training_hyperparameters=model_data["training_hyperparameters"],
	training_hyperparameters=model_data.get("training_hyperparameters", {}),

[codeowlai]

🐛 Correctness Issue

Removed Null Check Causes Potential Crash.

The code now directly accesses msg.content.result without checking if it's None first, which can cause a NullPointerException at runtime.

Current Code (Diff):

+                         if msg.content.result is None:
+                             # Handle null case appropriately
+                             continue
                          sms = msg.content.result

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	sms = msg.content.result
	if msg.content.result is None:
	# Handle null case appropriately
	continue
	sms = msg.content.result

[codeowlai]

🐛 Correctness Issue

Undefined Variable Reference.

The variable sms_all is only initialized within the QLib scenarios conditional block but is used unconditionally, causing an undefined variable error in non-QLib scenarios.

Current Code (Diff):

                         sms.name = f"Round {state.lround}"
-                         sms_all.name = f"Round {state.lround}"
                         state.metric_series.append(sms)
-                         state.all_metric_series.append(sms_all)
+                         if isinstance(state.scenario, (QlibModelScenario, QlibFactorFromReportScenario, QlibFactorScenario, QlibQuantScenario)):
+                             sms_all.name = f"Round {state.lround}"
+                             state.all_metric_series.append(sms_all)

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	sms.name = f"Round {state.lround}"
	sms_all.name = f"Round {state.lround}"
	state.metric_series.append(sms)
	state.all_metric_series.append(sms_all)
	sms.name = f"Round {state.lround}"
	if isinstance(state.scenario, (QlibModelScenario, QlibFactorFromReportScenario, QlibFactorScenario, QlibQuantScenario)):
	sms_all.name = f"Round {state.lround}"
	state.all_metric_series.append(sms_all)
	state.metric_series.append(sms)

[codeowlai]

🔒 Security Issue

Security: Expanded File Visibility.

This change broadens file visibility from model_*.py files to all Python files, potentially exposing sensitive configuration or credentials.

Current Code (Diff):

-                                if k.endswith(".py"):
+                                if k.startswith("model_") and k.endswith(".py"):

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	if k.endswith(".py"):
	if k.startswith("model_") and k.endswith(".py"):

[codeowlai]

🐛 Correctness Issue

Division by Zero Risk.

If all feedback lists are empty but non-zero in count, success_fb_trace_count will be zero, causing a division by zero error when calculating the slice size.

Current Code (Diff):

-             success_fb_trace_count = sum(1 for fb_list in trace_fbs if fb_list)
-             success_fb_list = [
-                 fb for fb_list in trace_fbs for fb in fb_list[-(num_to_slice // success_fb_trace_count) :]
-             ]
+             success_fb_trace_count = sum(1 for fb_list in trace_fbs if fb_list)
+             if success_fb_trace_count == 0:
+                 success_fb_list = []
+             else:
+                 success_fb_list = [
+                     fb for fb_list in trace_fbs for fb in fb_list[-(num_to_slice // success_fb_trace_count) :]
+                 ]

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	success_fb_trace_count = sum(1 for fb_list in trace_fbs if fb_list)
	success_fb_list = [
	fb for fb_list in trace_fbs for fb in fb_list[-(num_to_slice // success_fb_trace_count) :]
	]
	success_fb_trace_count = sum(1 for fb_list in trace_fbs if fb_list)
	if success_fb_trace_count == 0:
	success_fb_list = []
	else:
	success_fb_list = [
	fb for fb_list in trace_fbs for fb in fb_list[-(num_to_slice // success_fb_trace_count) :]
	]

[codeowlai]

🐛 Correctness Issue

Potential IndexError when accessing hypotheses[0].

Accessing hypotheses[0] will crash the application if the list is empty, causing a runtime exception.

Current Code (Diff):

-        exp = DSExperiment(pending_tasks_list=[[task]], hypothesis=hypotheses[0])
+        exp = DSExperiment(pending_tasks_list=[[task]], hypothesis=hypotheses[0] if hypotheses else None)

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	exp = DSExperiment(pending_tasks_list=[[task]], hypothesis=hypotheses[0])
	exp = DSExperiment(pending_tasks_list=[[task]], hypothesis=hypotheses[0] if hypotheses else None)

[codeowlai]

🐛 Correctness Issue

Missing Key Validation Causing Potential KeyError.

The code removed the previous validation check for 'problem' key, which could cause KeyError exceptions if any problem dictionary is missing this key.

Current Code (Diff):

  problem_formatted_str += f"## {i+1}. {problem_name}\n"
+ if "problem" not in problem_dict:
+     continue
  problem_formatted_str += f"{problem_dict['problem']}\n"

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	problem_formatted_str += f"## {i+1}. {problem_name}\n"
	problem_formatted_str += f"{problem_dict['problem']}\n"
	problem_formatted_str += f"## {i+1}. {problem_name}\n"
	if "problem" not in problem_dict:
	continue
	problem_formatted_str += f"{problem_dict['problem']}\n"

[codeowlai]

🐛 Correctness Issue

Duplicate Function Definition.

The preview_csv function is defined twice which will cause a Python syntax error and crash the application at runtime.

Current Code (Diff):

- def preview_csv(p: Path, file_name: str, simple=True, show_nan_columns=False) -> str:
-     """Generate a textual preview of a csv file"""
-     df = pd.read_csv(p)
-     return preview_df(df, file_name, simple=simple, show_nan_columns=show_nan_columns)

[codeowlai]

🐛 Correctness Issue

Breaking API Change - Removed Parameter Default.

Removing the default value for local_path parameter will break any existing code that calls this function without explicitly providing this parameter.

Current Code (Diff):

- def download_notebooks(competition: str, local_path: str, num: int = 15) -> None:
+ def download_notebooks(competition: str, local_path: str = f"{KAGGLE_IMPLEMENT_SETTING.local_data_path}/notebooks", num: int = 15) -> None:

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	def download_notebooks(competition: str, local_path: str, num: int = 15) -> None:
	def download_notebooks(competition: str, local_path: str = f"{KAGGLE_IMPLEMENT_SETTING.local_data_path}/notebooks", num: int = 15) -> None:

[codeowlai]

🐛 Correctness Issue

Removed default parameter value breaks backward compatibility.

Removing the default parameter value will cause runtime errors in all code that calls this function without explicitly providing the parameter.

Current Code (Diff):

- def collect_knowledge_texts(notebooks_path: str | Path) -> dict[str, list[str]]:
+ def collect_knowledge_texts(notebooks_path: str | Path = KAGGLE_IMPLEMENT_SETTING.local_data_path) -> dict[str, list[str]]:

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	def collect_knowledge_texts(notebooks_path: str | Path) -> dict[str, list[str]]:
	def collect_knowledge_texts(notebooks_path: str | Path = KAGGLE_IMPLEMENT_SETTING.local_data_path) -> dict[str, list[str]]:

🔄 Dependencies Affected

rdagent/scenarios/kaggle/kaggle_crawler.py

Function: Unknown function using collect_knowledge_texts

Issue: Any call to collect_knowledge_texts() without explicitly providing the parameter will fail with a missing argument error

Suggestion: Update all calls to provide the notebooks_path parameter explicitly

---

[codeowlai]

🐛 Correctness Issue

ValueError from non-integer filenames.

The sorting key function will raise ValueError if directory names or filenames don't follow the expected integer-based naming convention

Current Code (Diff):

-             files = sorted(path.glob("*/*_*"), key=lambda f: (int(f.parent.name), int(f.name.split("_")[0])))
-             path = files[-1]
+             try:
+                 files = sorted(path.glob("*/*_*"), key=lambda f: (int(f.parent.name), int(f.name.split("_")[0])))
+                 if not files:
+                     raise FileNotFoundError(f"No session files found in {path}")
+                 path = files[-1]
+             except ValueError:
+                 raise ValueError(f"Session files in {path} don't follow the expected naming convention (int/int_*)")

📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change

	files = sorted(path.glob("*/*_*"), key=lambda f: (int(f.parent.name), int(f.name.split("_")[0])))
	try:
	files = sorted(path.glob("*/*_*"), key=lambda f: (int(f.parent.name), int(f.name.split("_")[0])))
	if not files:
	raise FileNotFoundError(f"No session files found in {path}")
	path = files[-1]
	except ValueError:
	raise ValueError(f"Session files in {path} don't follow the expected naming convention (int/int_*)")