Main 436#437
Conversation
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
|
I only went thru the input/output type and docstring of each mcp tools, not implementation. My opinion is we should enhance the signature of mcp tools so that they are more well-defined and easy to understand by agents first. |
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
|
@ShuxinLin We adjusted all of your feedback 
|
| if sensor: | ||
| values = {sensor: doc.get(sensor)} | ||
| else: | ||
| values = {k: v for k, v in doc.items() if k not in RESERVED_FIELDS} |
There was a problem hiding this comment.
When sensor is omitted this returns only the fields present in the newest single document, not all measured sensors. With sparse telemetry it silently omits channels. Either require sensor, or return the latest value per measured sensor with per-sensor timestamps.
There was a problem hiding this comment.
Ideally sensor should be given. So please remove the else.
| "n_sensors": len(d.get("sensors", [])), | ||
| } | ||
| AssetSummary( | ||
| asset_id=d["assetnum"], |
There was a problem hiding this comment.
This returns assetnum as asset_id, while asset_ids() returns the telemetry-ready id (iot_asset_id or assetnum). Agents can chain assets() -> history() with CHILLER6 and miss Chiller 6 telemetry. Please return the canonical telemetry asset_id here and expose assetnum/wo_assetnum as aliases.
There was a problem hiding this comment.
Clarification: if iot_asset_id is present in the real asset registry, then assets() should include/use that value as the canonical telemetry asset_id and expose assetnum separately. If src/couchdb/scenarios_data/shared/iot/asset_profile_sample.json does not match the real registry shape, the sample fixture should be corrected too so local tests exercise the same ID contract.
There was a problem hiding this comment.
asset_id, assetnum --> in real scenarios IoT may represent asset via asset_id and wo has assetnum. but its mappping every real world dataset will have. So not sure what need to be done.
| return ErrorResult(error="provide at least one sensor name") | ||
|
|
||
| matches: List[AssetSensorMatch] = [] | ||
| for asset_id in _site_asset_ids(site_name): |
There was a problem hiding this comment.
_site_asset_ids() returns [] when asset_db is unavailable, so find_assets_by_sensors reports a successful 0 matches instead of a connection error. Please return ErrorResult when the required registry/telemetry DB is missing.
There was a problem hiding this comment.
Adjust if needed.
| return ErrorResult(error="CouchDB not connected") | ||
| limit = max(1, min(limit, PAGE_SIZE)) | ||
|
|
||
| selector = _time_selector(asset_id, start, end) |
There was a problem hiding this comment.
Telemetry tools still validate only that the site exists; the query is scoped by asset_id alone. This lets history(site_name="NORTH", asset_id="Chiller 6") return MAIN data. Please either enforce asset-site membership through the registry before querying, or remove site_name from telemetry tools.
There was a problem hiding this comment.
This issue, we actually already addressed a long time ago. Not sure why it is coming again.
| ) | ||
| for s in sorted(counts) | ||
| ] | ||
| return SensorCoverageResult( |
There was a problem hiding this comment.
Because max_scan_docs defaults to 5000, these coverage counts may be sampled, but the response has no truncated/has_more/max_scan_docs metadata. Please make partial coverage explicit so agents do not treat sampled counts as complete.
There was a problem hiding this comment.
Please add an extra flag indicating the sampled result.
|
@ShuxinLin, please work with @Miao900 to write the workflow now and make necessary changes in the tool. |
|
The new/changed IoT tool surface needs tests. The current focused suite fails locally ( |
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
Signed-off-by: Dhaval Patel <pateldha@us.ibm.com>
2 participants
No description provided.