Skip to content

Fix tool_instance_name resolution in save_parameters and support multiple TOPP tool instances#354

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-tool-instance-name-issue
Draft

Fix tool_instance_name resolution in save_parameters and support multiple TOPP tool instances#354
Copilot wants to merge 2 commits intomainfrom
copilot/fix-tool-instance-name-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 14, 2026

save_parameters passes tool instance names (e.g., "IDFilter_step1") directly to create_ini, which tries to execute them as binaries and fails. The instance name needs to be resolved back to the real tool name ("IDFilter") before calling create_ini.

Changes

  • StreamlitUI.input_TOPP — Added tool_instance_name parameter. Registers instance_name → tool_name mapping in st.session_state["_topp_tool_instance_map"]. Uses instance name for session state keys, real tool name for ini operations.

  • ParameterManager.save_parameters — Resolves instance names via the mapping before calling create_ini and loading ini files. Maps ini_key back to real tool name format for param.getValue() lookups. Stores params under instance name in JSON.

  • CommandExecutor.run_topp — Added tool_instance_name parameter for JSON param lookup, keeping real tool name for the executable and ini path.

  • ParameterManager.get_topp_parameters — Added optional tool_instance_name for instance-aware param retrieval.

Usage

# Two IDFilter instances with independent parameters
self.ui.input_TOPP("IDFilter", tool_instance_name="IDFilter_step1")
self.ui.input_TOPP("IDFilter", tool_instance_name="IDFilter_step2")

# Execution uses real tool name + instance name for param lookup
self.executor.run_topp("IDFilter", input_output={...}, tool_instance_name="IDFilter_step1")
self.executor.run_topp("IDFilter", input_output={...}, tool_instance_name="IDFilter_step2")

All parameters default to backward-compatible behavior when tool_instance_name is omitted.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Bug] Issues arising from tool_instance_name implementation: Parameter loading failure and command generation errors</issue_title>
<issue_description>The introduction of tool_instance_name to support multiple instances of the same TOPP tool (e.g., duplicate IDFilter calls) has caused a side effect when executing the save_parameters method.

Specifically, during the execution of the save_parameters loop, the tool_instance_name is being passed into the create_ini method. This results in an invalid tool name causing create_ini to fail.

for tool in current_topp_tools:
            if not self.create_ini(tool):
                # Could not create ini file - skip this tool
                continue
```</issue_description>

<agent_instructions>Please run a deep investigation why this is the case and open a PR with a summary, clearly tracing the error.</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] [Bug] Fix parameter loading failure in tool_instance_name implementation Fix tool_instance_name resolution in save_parameters and support multiple TOPP tool instances Mar 14, 2026
Copilot AI requested a review from t0mdavid-m March 14, 2026 18:38
Resolves the issue where multiple instances of the same TOPP tool could
not have independent custom defaults. The .ini file (keyed by tool name)
only supported one set of defaults — the first instance's defaults were
baked in and subsequent instances were silently ignored.

Changes:
- Add get_merged_params() to ParameterManager for centralized three-layer
  parameter resolution (ini defaults < _defaults < user overrides)
- Replace .ini file mutation with _defaults seeding in params.json,
  keyed by tool instance name
- Update save_parameters() to compare against _defaults when determining
  which values are non-default
- Update run_topp() and non_default_params_summary() to use merged params
- Resolve instance names to real tool names across save_parameters,
  input_TOPP, run_topp, and get_topp_parameters
@t0mdavid-m t0mdavid-m force-pushed the copilot/fix-tool-instance-name-issue branch from 2d23b57 to 5949251 Compare April 7, 2026 11:31
The .ini file is no longer mutated with custom defaults — it stays
pristine. All parameters (_defaults + user overrides) are already
passed as CLI flags via get_merged_params(), making -ini redundant.
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.

[Bug] Issues arising from tool_instance_name implementation: Parameter loading failure and command generation errors

2 participants