zxs1633079383 commented on PR #870: URL: https://github.com/apache/flink-agents/pull/870#issuecomment-4934131899
@joeyutong @weiqingy Thanks, I agree with narrowing this PR. I pushed 5af8560, which removes automatic embedding token metric recording and leaves this PR focused on provider usage extraction plus the `EmbeddingResult` / `embedWithUsage(...)` and Python equivalent APIs. The vector-store paths are back to the existing `embed(...)` API. That restores the public override point, and model resolution is again lazy inside the missing-embedding branch, so Mem0 stores without a configured embedding model keep their prior behavior. This directly addresses the Python CI failures you pointed out. I also agree that pre-resolving handles would only address the lazy lookup race, not the broader question of mutating counters from arbitrary resource threads. I will not pursue a `ThreadSafeSimpleCounter` or embedding-specific recorder in this PR; a framework-level safe metric/effect boundary should be designed separately. Local verification for the affected paths is green: 49 Python tests (including Chroma and Mem0 vector stores), the API embedding-result test, and the Bedrock batch-usage test. CI should now rerun on the narrowed patch. Thanks again for steering this toward a cleaner boundary. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
