Skip to content

Conversation

@HuawenShen
Copy link
Collaborator

Description

How Has This Been Tested?

  • Unit tests (pytest tests/)
  • Integration tests (if applicable)
  • If there is a new feature, please describe how you test it:
    MCP, job queue: manually tested

Configuration Changes

  • No config changes
  • Config changes have been updated in default.yaml. Please describe the changes:

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Performance improvement
  • Code cleanup/refactor

@HuawenShen HuawenShen requested a review from FANGAreNotGnu July 9, 2025 18:15
Copy link
Collaborator

@FANGAreNotGnu FANGAreNotGnu left a comment

Choose a reason for hiding this comment

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

Left some comments for the backend logics, will go through webui and mcp changes later today


```bash
mlzero -i INPUT_DATA_FOLDER [-o OUTPUT_DIR] [-c CONFIG_PATH] [-n MAX_ITERATIONS] [--need-user-input] [-u INITIAL_USER_INPUT] [-e EXTRACT_TO] [-v VERBOSITY_LEVEL]
mlzero -i INPUT_DATA_FOLDER [-o OUTPUT_DIR] [-c CONFIG_PATH] [-n MAX_ITERATIONS] [--ENABLE-PER-ITERATION-INSTRUCTION] [--INITIAL-INSTRUCTION] [-e EXTRACT_TO] [-v VERBOSITY_LEVEL]
Copy link
Collaborator

Choose a reason for hiding this comment

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

add an abbreviation for --INITIAL-INSTRUCTION, e.g. -t

need_user_input: bool = typer.Option(
False,
"--enable-per-iteration-instruction",
help="If enabled, you can provide a prompt for the next iteration at each iteration",
Copy link
Collaborator

Choose a reason for hiding this comment

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

If enabled, provide an instruction at the start of each iteration (except the first, which uses the initial instruction). The process suspends until you provide it.

Comment on lines 15 to 17
from pathlib import Path # noqa: E402

import typer # noqa: E402
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move it to the correct place?

Comment on lines 21 to 23
from .. import __file__ as assistant_file # noqa: E402

PACKAGE_ROOT = Path(assistant_file).parent
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason not to write as Path(__file__).parent.parent?

from omegaconf import OmegaConf

from .managers import Manager
# from .managers import Manager
Copy link
Collaborator

Choose a reason for hiding this comment

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

Delete the code instead of commenting out

"""Cleanup method to properly close the embedding model."""
self.cleanup()

def __log_wrapper(self, input):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use a better name, e.g. _silent_encode

Copy link
Collaborator

@FANGAreNotGnu FANGAreNotGnu left a comment

Choose a reason for hiding this comment

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

Had two minor comments to fix. Also let's discuss offline about the position of the mcp folder

.gitignore Outdated
*.credentials
*.creds
credentials.txt
aws_credentials.txt No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

EOF

run_agent(
input_data_folder=<your-input-folder>,
output_folder=<your-output-folder>
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

add a commented line:

run_agent(
      input_data_folder=<your-input-folder>,
      output_folder=<your-output-folder>,
      # more args ...
)

@FANGAreNotGnu
Copy link
Collaborator

Also it seems only test_titanic_prediction is tested in CI

Copy link
Collaborator

@FANGAreNotGnu FANGAreNotGnu left a comment

Choose a reason for hiding this comment

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

LGTM. We should add more unit/integrated tests in the following PRs.

@FANGAreNotGnu FANGAreNotGnu merged commit d81eba4 into autogluon:main Jul 11, 2025
3 checks passed
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.

2 participants