fix: background-job response hygiene — no traceback leaks, honest ETA (#538) - #550
Closed
Shashankss1205 wants to merge 1 commit into
Closed
Shashankss1205 wants to merge 1 commit into
Shashankss1205 wants to merge 1 commit into
Conversation
…#538) - NB-04: async job errors[] stored [str(e), traceback.format_exc()], leaking the full server traceback with absolute filesystem paths to the client. Store only the clean message and log the traceback server-side. Also stripped the "traceback" key from sync fit/predict/call_method error responses (same leak), and aligned the async "Unknown metric" message with the sync hint. - NB-05: estimated_time_remaining extrapolated from 2-3 coarse steps and was wildly wrong (15s remaining when a job finished in <1s). Return None unless the job reports >= 10 steps. A cancelled job now sets current_step="Cancelled" instead of freezing on the last in-flight label. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Closing: this PR's content is already on main as commit f8f377c (identical patch-id; it was landed directly rather than through the PR merge button). Nothing further to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #538. Background-job response hygiene:
NB-04 — errors[] leaked full server tracebacks
The async failure paths stored
errors=[str(e), traceback.format_exc()], exposing the full traceback with absolute filesystem paths to the MCP client. Now they store only the clean message and log the traceback server-side. The same"traceback"key was also stripped from the sync fit/predict/call_method error responses (same leak), and the async "Unknown metric" message now carries the samequery_registry(task='metric')hint as the sync path.Verified end-to-end: an async fit with a bad handle returns
errors: ["Handle not found: est_bogus_zzz"]— noTraceback, no.pypaths.NB-05 — misleading ETA and stale terminal step
estimated_time_remainingextrapolated from 2–3 coarse steps and was wildly wrong (reported ~15s remaining when a job finished in under a second). It now returnsNoneunless the job reports ≥10 steps, so coarse jobs stop faking an estimate.current_step = "Cancelled"instead of freezing on the last in-flight label (e.g. "Running cross-validation...").Testing
tests/test_job_response_quality.py(4 tests): no ETA for coarse jobs, ETA present for fine-grained jobs, cancel sets a terminal step + no ETA, failed-job errors carry no traceback/paths.🤖 Generated with Claude Code