Skip to content

refactor(fmsr): drop LLM generation, fix asset_class matching - #552

Open
ShuxinLin wants to merge 4 commits into
mainfrom
refactor/drop-fmsr-generate-tool
Open

ShuxinLin wants to merge 4 commits into
mainfrom
refactor/drop-fmsr-generate-tool

Conversation

@ShuxinLin

@ShuxinLin ShuxinLin commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Simplifies the FMSR MCP server so it only stores and looks up failure-mode catalogs. It no longer calls an LLM of its own. This PR also fixes how asset classes are matched.

1. Drop generate_failure_modes. The tool called a separate LLM inside the server (FMSR_MODEL_ID, default WatsonX Llama-3.3-70B). Because of that:

  • fmsr results depended on that fixed model instead of the agent being tested;
  • without WatsonX credentials the tool quietly returned {"error": "LLM unavailable"}, so lite-profile fmsr scenarios scored badly and no error was shown;
  • the server needed its own LLM credentials and billing.

Agents now come up with failure modes from their own domain knowledge and save them with add_failure_modes. The failure-mode/sensor mapping already works this way. MCP sampling was considered, but none of the three runners support it yet.

2. Fix asset-class matching (merged in from #553).

  • Stored names with capitals or punctuation could not be found. The loader stores records as fm:<raw asset_class>, while the server lowercased the input and looked up fm:<key>. Records such as "Hydraulic Pump" were never found, and add_failure_modes created duplicates of them.
  • Digits were stripped from class names. CO2 Compressor became co compressor, and 3-Phase Motor became phase motor.

Changes

  • Documentation / Tutorial update
  • Refactoring (no logic change): not quite; this removes a tool and changes matching

Server (src/servers/fmsr/main.py)

  • Removed: generate_failure_modes, its result model, the prompts and the LLM setup (FMSR_MODEL_ID). The server now makes no LLM calls.
  • Matching: records are matched on asset_class only, never _id. Input and stored names go through the same normalisation, which ignores case, punctuation, _ and whitespace and leaves digits alone.
  • Ties: if several records share the same normalised name, the lowest _id wins.
  • Errors: a not-found error suggests close matches ("Did you mean: Hydraulic Pump?") and lists the available classes.
  • Updates: add_failure_modes keeps the stored asset_class name. New records are created as fm:<normalised name>.
  • Tool descriptions: they tell agents to pass a generic equipment type, not an asset id ("Chiller 6" → "chiller"), to retry with a suggested class, and to reuse existing class names.
  • Server instructions: they now say the agent generates failure modes and mappings itself and saves them with add_failure_modes.

Tests (src/servers/fmsr/tests/)

  • Removed: the generate-tool tests and fixtures. The registration test checks that the tool set is exactly {get_failure_modes, add_failure_modes}.
  • Test database stand-in: FakeDatabase.find now returns copies. It used to return the stored dicts themselves, so write tests passed even if save never ran. It also rejects conflicting writes (_rev) the way CouchDB does.
  • New tests:
    • stored names with capitals or punctuation, and _ids not in the fm: form
    • digits kept in class names
    • instance ids rejected, with a suggestion
    • _id never used for matching
    • duplicate normalised classes
    • updates applied to the matching record
    • a failed save
    • a conflicting concurrent update

Docs and scripts

  • README: tool table.
  • docs/mcp-servers.md: tool list and requirements.
  • docs/running_benchmark.md: removed the FMSR_MODEL_ID row, the "WatsonX — needed more often than it looks" section and the related troubleshooting row.
  • docs/opencode-agent.md: removed the generate smoke tests and renumbered the rest.
  • scripts/preflight_run.sh: removed the WatsonX check.

Impact on benchmarking

  • Credentials: fmsr scenarios no longer need WatsonX credentials.
  • Class-name input: pump, Pump and hydraulic_pump give the same results as before for the bundled pump sample.
  • Instance ids: inputs such as Pump-1 no longer map to pump. They now get a not-found error that suggests pump.
  • Not run: no before/after benchmark comparison. The scenario questions aren't in this repo, so I couldn't check whether any of them tell the agent to call generate_failure_modes. Local reports/ for 902–916 show answers taken from the catalog.

Testing

  • uv run pytest src/servers/fmsr -q passes (32 tests).
  • uv run pytest src/ -k "not integration": 19 failures, the same set that fails on main (iot test_invalid_site, test_static_json_scorer, test_file_exporter). None are in fmsr.
  • ruff format: clean. ruff check still reports the file's existing List/Optional typing-style warnings.

Not in this PR

  • notebook/kdd_tutorial/03_mcp_fmsr_fixed.ipynb still references generate_failure_modes. It is untracked locally.

Checklist

  • I have signed off my commits (DCO).

The tool called a separate server-side LLM (FMSR_MODEL_ID, default
WatsonX Llama-3.3-70B), so fmsr scenario results depended on that model
rather than the agent under test, and silently degraded to
'LLM unavailable' without WatsonX credentials.

Agents now derive failure modes from their own domain knowledge and
persist them with add_failure_modes, matching how the failure-mode/sensor
mapping is already handled. Removes the LLM backend, prompts, related
tests/fixtures, the FMSR_MODEL_ID docs, and the preflight WatsonX check.

Signed-off-by: Shuxin Lin <linshuhsin@gmail.com>
- Look up records by comparing normalised asset_class values in Python
  instead of by _id. The loader keys docs as fm:<raw asset_class>, so
  classes like 'Hydraulic Pump' were unreachable by the lowercased key.
- Normalise only case, punctuation and whitespace; stop stripping digits,
  which turned 'CO2 Compressor' into 'co compressor'.
- Keep the stored asset_class name when updating a record.
- Suggest close matches in the not-found error.

Tests: FakeDatabase now returns copies from find() and enforces _rev
conflicts like CouchDB; add coverage for write failures, concurrent
updates, duplicate normalised classes and non-fm: _ids.

Signed-off-by: Shuxin Lin <linshuhsin@gmail.com>
Signed-off-by: Shuxin Lin <linshuhsin@gmail.com>
fix(fmsr): match failure-mode records on asset_class only
@ShuxinLin ShuxinLin changed the title refactor(fmsr): drop generate_failure_modes tool refactor(fmsr): drop LLM generation, fix asset_class matching Sep 25, 2026

This branch has not been deployed

No deployments
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.

1 participant