refactor: mcp only uses mdr for schemas #821
Conversation
cbeach47
left a comment
cbeach47
left a comment
There was a problem hiding this comment.
Smoke test using the Advisor looked good. I noticed the following on start up of the MCP. Is the warning for Organization expected?
2026-01-30 14:35:52.715 INFO | lif.schema_state_manager.core | Initializing SchemaStateManager (sync)...
2026-01-30 14:35:52.715 INFO | lif.mdr_client.core | Fetching OpenAPI schema from MDR: 17
2026-01-30 14:35:53.024 INFO | httpx | HTTP Request: GET [http://lif-mdr-api:8012/datamodels/open_api_schema/17?include_attr_md=true&include_entity_md=false](http://lif-mdr-api:8012/datamodels/open_api_schema/17?include_attr_md=true&include_entity_md=false) "HTTP/1.1 200 OK"
2026-01-30 14:35:53.025 INFO | lif.mdr_client.core | Successfully loaded OpenAPI schema from MDR
2026-01-30 14:35:53.036 INFO | lif.schema_state_manager.core | Loaded 186 schema leaves for root 'Person'
2026-01-30 14:35:53.045 INFO | lif.schema_state_manager.core | Loaded 14 schema leaves for root 'Course'
2026-01-30 14:35:53.052 ERROR | lif.openapi_schema_parser.core | Root schema 'Organization' not found. Available: ['Assessment', 'Course', 'Credential', 'OperationalStatus', 'OrganizationCode', 'Person', 'Position', 'Program', 'RemunerationPackage']
2026-01-30 14:35:53.052 WARNING | lif.schema_state_manager.core | Failed to load schema leaves for optional root 'Organization': Root schema 'Organization' not found.
2026-01-30 14:35:53.060 INFO | lif.schema_state_manager.core | Loaded 47 schema leaves for root 'Credential'
Maybe a separate ticket, but I'd love to see an integration test where:
- MCP is queried and MDR data is pulled
- MDR data is changed
- MCP is queried and the update MDR data is pulled
Approving for a static review (skimmed the tests), and the smoke test with the default configuration. Is there a scenario you'd recommend to test that a change in MDR propagates into the MCP and the Advisor would adjust it's response?
|
|
||
| Note: This updates the internal state but does NOT update the MCP tool | ||
| definitions, as those are set at module load time. To use new schema | ||
| definitions for tools, the server must be restarted. |
There was a problem hiding this comment.
Is this a phased approach?
Seems less than ideal to require a restart just so lif_query() or lif_mutation() would pick up the refreshed schema.
| Returns: | ||
| Tuple of (openapi_dict, source) or (None, source) on failure | ||
| """ | ||
| # For now, delegate to sync version since load_openapi_schema is sync |
There was a problem hiding this comment.
What is the impact to MCP that this is only sync for now?
| for _ in range(10): | ||
| _ = manager.state | ||
| _ = manager.is_initialized | ||
| _ = manager.get_status() |
There was a problem hiding this comment.
is this really testing thread safety? Appears to be sequential access...
2 participants
Checklist
uv run ruff check)uv run ruff format)uv run ty check)docs/and project README updated
and CHANGELOG.md entry
Type of Change
Refactor Semantic Search MCP Server to only use MDR as a schema source
to not work as expected)
Description of Change
Related Issues
Resolves #782
Closes # [[add Github issue number]]
Testing
Project Area(s) Affected
Additional Notes