Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,26 @@ taskflow:

The model version can then be updated by changing `gpt_latest` in the `model_config` file and applied across all taskflows that use the config.

In addition, model specific parameters can be provided via `model_config`. To do so, define a `model_settings` section in the `model_config` file. This section has to be a dictionary with the model names as keys:

```yaml
model_settings:
gpt_latest:
temperature: 1
reasoning:
effort: high
```

You do not need to set parameters for all models defined in the `models` section. When parameters is not set for a model, they'll fall back to the default value. However, all the settings in this section must belong to one of the models specified in the `models` section, otherwise an error will raise:
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

Grammatical error: "an error will raise" should be "an error will be raised" (passive voice) or "the system will raise an error" (active voice).

Suggested change
You do not need to set parameters for all models defined in the `models` section. When parameters is not set for a model, they'll fall back to the default value. However, all the settings in this section must belong to one of the models specified in the `models` section, otherwise an error will raise:
You do not need to set parameters for all models defined in the `models` section. When parameters is not set for a model, they'll fall back to the default value. However, all the settings in this section must belong to one of the models specified in the `models` section, otherwise an error will be raised:

Copilot uses AI. Check for mistakes.

```yaml
model_settings:
new_model:
...
```

The above will result in an error because `new_model` is not defined in `models` section. Model parameters can also be set per task, and any settings defined in a task will override the settings in the config.

## Passing environment variables

Files of types `taskflow` and `toolbox` allow environment variables to be passed using the `env` field:
Expand Down
12 changes: 12 additions & 0 deletions doc/GRAMMAR.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ Tasks can optionally specify which Model to use on the configured inference endp

Note that model identifiers may differ between OpenAI compatible endpoint providers, make sure you change your model identifier accordingly when switching providers. If not specified, a default LLM model (`gpt-4o`) is used.

Parameters to the model can also be specified in the task using the `model_settings` section:

```yaml
model: gpt-5-mini
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The example uses gpt-5-mini which may not be a valid model identifier. As of January 2025, there is no GPT-5 model released by OpenAI. Consider using a more realistic example model name like gpt-4o-mini or a placeholder like your-model-name to avoid confusion.

Suggested change
model: gpt-5-mini
model: gpt-4o-mini

Copilot uses AI. Check for mistakes.
model_settings:
temperature: 1
reasoning:
effort: high
```

If `model_settings` is absent, then the model parameters will fall back to either the default or the ones supplied in a `model_config`. However, any parameters supplied in the task will override those that are set in the `model_config`.

### Completion Requirement

Tasks can be marked as requiring completion, if a required task fails, the taskflow will abort. This defaults to false.
Expand Down
36 changes: 28 additions & 8 deletions src/seclab_taskflow_agent/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
exclude_from_context: bool = False,
max_turns: int = DEFAULT_MAX_TURNS,
model: str = DEFAULT_MODEL,
model_settings: ModelSettings | None = None,
model_par: dict = {},
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The parameter name model_par is inconsistent with the naming convention used elsewhere in the codebase. The function uses model_settings internally (line 137) and the parameter being passed from the caller is called model_settings (line 644). Consider renaming this parameter to model_params or model_parameters for better clarity and consistency.

Copilot uses AI. Check for mistakes.
run_hooks: TaskRunHooks | None = None,
agent_hooks: TaskAgentHooks | None = None):

Expand All @@ -130,10 +130,11 @@

# https://openai.github.io/openai-agents-python/ref/model_settings/
parallel_tool_calls = True if os.getenv('MODEL_PARALLEL_TOOL_CALLS') else False
model_settings = ModelSettings(
temperature=os.getenv('MODEL_TEMP', default=0.0),
tool_choice=('auto' if toolboxes else None),
parallel_tool_calls=(parallel_tool_calls if toolboxes else None))
model_params = {'temperature' : os.getenv('MODEL_TEMP', default = 0.0),
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

Inconsistent spacing around the = operator. Line 133 uses default = 0.0 with spaces, which deviates from PEP 8 style guidelines. It should be default=0.0 without spaces around the equals sign in function arguments.

Suggested change
model_params = {'temperature' : os.getenv('MODEL_TEMP', default = 0.0),
model_params = {'temperature' : os.getenv('MODEL_TEMP', default=0.0),

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The os.getenv('MODEL_TEMP', default=0.0) call returns a string, not a float. Environment variables are always strings, so this will pass "0.0" (a string) to ModelSettings instead of the numeric value 0.0. This should be float(os.getenv('MODEL_TEMP', default='0.0')) or use default=0.0 only after converting the environment variable value to a float.

Suggested change
model_params = {'temperature' : os.getenv('MODEL_TEMP', default = 0.0),
model_params = {'temperature' : float(os.getenv('MODEL_TEMP', default='0.0')),

Copilot uses AI. Check for mistakes.
'tool_choice' : ('auto' if toolboxes else None),
'parallel_tool_calls' : (parallel_tool_calls if toolboxes else None)}
Comment on lines +133 to +135
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

Inconsistent spacing around the assignment operator. The code uses 'temperature' : os.getenv(...) with spaces around the colon, which is not standard Python style. According to PEP 8, dictionary keys should not have spaces around the colon. This should be 'temperature': os.getenv(...) (and similarly for the other keys).

Suggested change
model_params = {'temperature' : os.getenv('MODEL_TEMP', default = 0.0),
'tool_choice' : ('auto' if toolboxes else None),
'parallel_tool_calls' : (parallel_tool_calls if toolboxes else None)}
model_params = {'temperature': os.getenv('MODEL_TEMP', default = 0.0),
'tool_choice': ('auto' if toolboxes else None),
'parallel_tool_calls': (parallel_tool_calls if toolboxes else None)}

Copilot uses AI. Check for mistakes.
model_params.update(model_par)
model_settings = ModelSettings(**model_params)

# block tools if requested
tool_filter = create_static_tool_filter(blocked_tool_names=blocked_tools) if blocked_tools else None
Expand Down Expand Up @@ -438,13 +439,22 @@
global_variables.update(cli_globals)
model_config = taskflow.get('model_config', {})
model_keys = []
models_params = {}
if model_config:
model_dict = available_tools.get_model_config(model_config)
model_dict = model_dict.get('models', {})
m_config = available_tools.get_model_config(model_config)
model_dict = m_config.get('models', {})
if model_dict:
if not isinstance(model_dict, dict):
raise ValueError(f"Models section of the model_config file {model_config} must be a dictionary")
model_keys = model_dict.keys()
model_keys = model_dict.keys()
models_params = m_config.get('model_settings', {})
if models_params and not isinstance(models_params, dict):
raise ValueError(f"Settings section of model_config file {model_config} must be a dictionary")
if not set(models_params.keys()).difference(model_keys).issubset(set([])):
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The validation logic if not set(models_params.keys()).difference(model_keys).issubset(set([])) is unnecessarily complex. The condition .issubset(set([])) is equivalent to checking if the set is empty. This should be simplified to if set(models_params.keys()) - set(model_keys): or if not set(models_params.keys()).issubset(model_keys): for better readability.

Suggested change
if not set(models_params.keys()).difference(model_keys).issubset(set([])):
if not set(models_params.keys()).issubset(model_keys):

Copilot uses AI. Check for mistakes.
raise ValueError(f"Settings section of model_config file {model_config} contains models that are not in the model section")
Comment on lines +453 to +454
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

The error message says "Settings section...contains models that are not in the model section" but should say "models that are not in the models section" (with an 's' on 'models'). Additionally, this error message could be more helpful by indicating which models are problematic. Consider: f"Settings section of model_config file {model_config} contains models that are not defined in the models section: {', '.join(set(models_params.keys()) - set(model_keys))}"

Suggested change
if not set(models_params.keys()).difference(model_keys).issubset(set([])):
raise ValueError(f"Settings section of model_config file {model_config} contains models that are not in the model section")
problematic_models = set(models_params.keys()) - set(model_keys)
raise ValueError(
f"Settings section of model_config file {model_config} contains models that are not defined in the models section: {', '.join(problematic_models)}"
)

Copilot uses AI. Check for mistakes.
for k,v in models_params.items():
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

Missing space after comma in for k,v in models_params.items():. According to PEP 8, there should be a space after the comma: for k, v in models_params.items():.

Suggested change
for k,v in models_params.items():
for k, v in models_params.items():

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Missing space after comma in the loop. Should be for k, v in instead of for k,v in according to PEP 8 style guidelines.

Suggested change
for k,v in models_params.items():
for k, v in models_params.items():

Copilot uses AI. Check for mistakes.
if not isinstance(v, dict):
raise ValueError(f"Settings for model {k} in model_config file {model_config} is not a dictionary")

for task in taskflow['taskflow']:

Expand All @@ -465,10 +475,19 @@
if k not in task_body:
task_body[k] = v
model = task_body.get('model', DEFAULT_MODEL)
model_settings = {}
if model in model_keys:
if model in models_params:
model_settings = models_params[model]
model = model_dict[model]
task_model_settings = task_body.get('model_settings', {})
if not isinstance(task_model_settings, dict):
name = task.get('name', '')
raise ValueError(f"model_settings in task {name} needs to be a dictionary")
model_settings.update(task_model_settings)

# parse our taskflow grammar
name = task_body.get('name', 'taskflow') # placeholder, not used yet

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'name' is unnecessary as it is
redefined
before this value is used.
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

This assignment to 'name' is unnecessary as it is redefined before this value is used.

Suggested change
name = task_body.get('name', 'taskflow') # placeholder, not used yet

Copilot uses AI. Check for mistakes.
description = task_body.get('description', 'taskflow') # placeholder not used yet
agents = task_body.get('agents', [])
headless = task_body.get('headless', False)
Expand Down Expand Up @@ -622,6 +641,7 @@
on_tool_end=on_tool_end_hook,
on_tool_start=on_tool_start_hook),
model = model,
model_par = model_settings,
agent_hooks=TaskAgentHooks(
on_handoff=on_handoff_hook))
return result
Expand Down