Add optional automatic tool naming by noobydp · Pull Request #51 · tracefinity/tracefinity

noobydp · 2026-05-31T12:48:26Z

Summary

Add optional automatic naming for traced tool polygons through a small ToolNamer abstraction.
Keep naming disabled by default; the only adapter in this PR is local Ollama.
Apply names synchronously after tracing and before session persistence, preserving generic tool N labels when naming is unavailable or returns an unusable result.
Keep the trace UI simple: ordinary editable polygon labels, no naming status, polling, retry endpoint, warmup task, or background scheduler.
Document the feature lightly in the README and move setup/details to docs/tool-naming.md.

Why

This keeps automatic naming optional and pluggable without making the trace workflow depend on model availability or hosted providers. The trace result remains immediately usable with generic labels, and naming failures do not block saving tools.

Validation

python -m pytest backend/tests/test_ort_runtime.py - passed, 6 tests.
python -m pytest backend/tests/test_tool_namer.py - passed, 12 tests.
python -m pytest backend/tests - collection reached 172 tests, then stopped on an unrelated existing collection error in backend/tests/test_image_downscale.py: it imports _downscale_image from app.api.routes, which is not exported on this branch/base.
git diff --check - passed.
Searched for the split-out ONNX tuning variables and helpers; no remaining references to TRACEFINITY_ONNX_GPU_MEM_LIMIT_MB, TRACEFINITY_ONNX_ARENA_EXTEND_STRATEGY, or their helper functions.

Codex Disclaimer

Code and PR drafted with Codex

noobydp · 2026-06-03T04:38:39Z

Update after conflict-reduction pass:

Added Reduce auto-naming merge conflicts (e18347a).
Moved background naming task logic out of routes.py into backend/app/services/tool_label_tasks.py, reducing route-file churn while keeping the same naming behavior.
Adjusted the trace-page tool list to preserve the existing pencil/edit-label interaction style while retaining naming status, cancel, retry, polling, and save-time cancellation behavior.
Made the API docs additive around automatic naming behavior.

Validation for this follow-up:

backend\venv\Scripts\python.exe -m py_compile backend\app\api\routes.py backend\app\services\tool_label_tasks.py backend\app\services\tool_labeler.py backend\tests\test_tool_labeler.py
frontend\node_modules\.bin\tsc.cmd -p frontend\tsconfig.json --noEmit

Note: pytest was not available in this worktree venv or system Python during this follow-up check.

Pairwise merge status after this update:

Clean with Add filleted rectangle cutouts #54 filleted rectangles.
Still structurally conflicts with Add photo station reuse workflow #53 photo station in shared trace workflow files; resolving that cleanly likely needs stacking or a shared base PR.

noobydp · 2026-06-03T07:21:14Z

The E2E failure is caused by a selector expectation that no longer matches the trace selection UI.

The failing test is frontend/e2e/happy-path.spec.ts in verify trace results, where it looks for buttons named /^Include tool \d+$/. This PR changed that sidebar row so the whole detected-tool row is the include/select control, while the only nested button is now the pencil button for editing the tool name (aria-label="Edit ..."). That was intentional: the UI now keeps the existing pencil/edit-label interaction while adding automatic naming status and editable generated labels.

Could the test be updated to select tools through the current row interaction, or better, through a stable test id / accessible selector that represents the selectable tool row instead of assuming each include control is a button named Include tool N?

Code and PR drafted with Codex.

jasonmadigan

thanks for putting this together. the implementation is thorough and the UX flow (generic labels first, stream in names, allow manual override) is well thought out.

a few things to discuss before we move forward on the code:

direction concern

tool naming/categorisation should be optional and pluggable, not tied to a specific inference approach. this PR bakes in four provider backends (Ollama, Gemini, OpenRouter, hosted fallback) with warmup, polling, retry, and background task infrastructure. that's a lot of surface area for what should be one possible implementation behind a simple interface. I'd rather see a clean abstraction (e.g. a ToolNamer protocol with a single name(image) -> str method) that this ML approach can plug into, alongside simpler alternatives (manual-only, filename-based, etc).

code-level findings (for when the direction is agreed)

3 of 21 new tests fail -- test_background_labeling_* tests pass "default" (string) where SessionStore is expected. the outer try/except swallows the AttributeError, so some tests pass by accident
asyncio.create_task() with no stored reference -- task can be silently GC'd
multiple ToolLabeler() instantiations per request (re-reads settings each time)
os.getenv() alongside pydantic-settings creates double source of truth for config
500ms polling (40 attempts) -- consider SSE or a lightweight status endpoint

happy to discuss the architecture direction. I've added a DESIGN.md to capture these kinds of decisions so we're aligned before PRs get too far along.

noobydp · 2026-06-06T16:05:01Z

Hey, sorry for this one. I intended to keep it optional/pluggable but the implementation got complicated over time and I didn't consider how impactful it was by the time I finished.

I'll rework this and follow your direction and fix those other things.

Do you want me to share extra info before I re-submit a PR, or push to my fork or something?

Reworks the rejected upstream PR tracefinity#51 to address the maintainer's direction and code-level feedback. Naming is now a small pluggable capability instead of bespoke multi-provider background infrastructure: - `ToolNamer` protocol (`name(image) -> str | None`) with a single `GeminiToolNamer` implementation that reuses the existing Gemini / OpenRouter label model. Any namer (local VLM, heuristic, test stub) can plug in without touching callers. - `get_tool_namer()` reads app config (pydantic-settings) as the single source of truth — no `os.getenv` second source. - On-demand `POST /sessions/{id}/name-tools` endpoint names crops synchronously with `asyncio.gather` — no background `create_task` (no GC risk), no warm-up, no 500ms polling. One namer per request. - Frontend: a user-triggered "Auto-name tools" button in the Select Tools step (shown only when a backend is available), plus inline editable tool names so users can correct before saving. Naming is strictly optional: the button only appears when a Gemini / OpenRouter key is available, and failures are swallowed so the trace flow is never blocked. Adds real unit tests for name normalisation, backend selection, the protocol, and error handling. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

noobydp · 2026-06-21T12:23:27Z

I reworked this to follow the optional/pluggable direction more closely.

The branch now removes the provider-heavy/background naming surface and keeps the PR focused on a small foundation:

ToolNamer protocol with fallback/no-op behavior by default
optional local Ollama adapter only
no Gemini/OpenRouter/hosted naming in this PR
no background scheduler, warmup task, retry endpoint, status fields, or polling UI
trace and mask-trace keep generic labels if naming is disabled or fails
README only mentions the opt-in feature briefly; setup details moved to docs/tool-naming.md

I also squashed the branch to one commit over current upstream/main and reran the focused backend/frontend checks listed in the PR body.

jasonmadigan

nice rework, much cleaner. one thing worth fixing:

# Conflicts: # backend/app/api/routes.py

noobydp · 2026-07-11T06:26:33Z

Updated the branch for the latest main and addressed the requested timeout cleanup.

Changes:

Removed the duplicate asyncio.wait_for(...) wrapper in OllamaToolNamer.name(). The existing httpx.AsyncClient(timeout=...) now owns the request timeout.
Merged current upstream/main into the branch to resolve the PR conflict.
Resolved the only conflict in backend/app/api/routes.py by keeping both upstream's route imports and this PR's name_polygons import.

Validation:

python -m pytest backend/tests/test_tool_namer.py backend/tests/test_ort_runtime.py backend/tests/test_image_downscale.py - passed, 19 tests.
python -m pytest backend/tests - ran 247 tests: 239 passed, 8 failed. The remaining failures are the existing Windows-local chmod(0o000) permission expectations in backend/tests/test_store_load_errors.py; they are unrelated to automatic tool naming.
git diff --check - passed, with Windows CRLF normalization warnings only.
Verified the branch now merges cleanly with upstream/main.

Code and PR drafted with Codex

noobydp changed the title ~~[codex] Add optional automatic tool naming~~ Add optional automatic tool naming May 31, 2026

noobydp mentioned this pull request May 31, 2026

Add photo station reuse workflow #53

Closed

noobydp marked this pull request as ready for review May 31, 2026 15:43

noobydp mentioned this pull request Jun 3, 2026

Add filleted rectangle cutouts #54

Merged

jasonmadigan requested changes Jun 3, 2026

View reviewed changes

noobydp marked this pull request as draft June 21, 2026 11:57

noobydp force-pushed the codex/auto-tool-names branch 2 times, most recently from 3f4fe52 to 3d1b156 Compare June 21, 2026 12:17

Add optional automatic tool naming

c9ef7b5

noobydp force-pushed the codex/auto-tool-names branch from 3d1b156 to c9ef7b5 Compare June 22, 2026 05:29

jasonmadigan approved these changes Jun 22, 2026

View reviewed changes

Comment thread backend/app/services/tool_namer.py

noobydp added 2 commits July 11, 2026 14:21

Remove duplicate tool naming timeout

db70eaa

Merge remote-tracking branch 'upstream/main' into codex/auto-tool-names

09c03c8

# Conflicts: # backend/app/api/routes.py

noobydp marked this pull request as ready for review July 11, 2026 06:32

jasonmadigan merged commit 7fd98bd into tracefinity:main Jul 16, 2026
3 checks passed

noobydp deleted the codex/auto-tool-names branch July 17, 2026 05:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add optional automatic tool naming#51

Add optional automatic tool naming#51
jasonmadigan merged 3 commits into
tracefinity:mainfrom
noobydp:codex/auto-tool-names

noobydp commented May 31, 2026 •

edited

Loading

Uh oh!

noobydp commented Jun 3, 2026

Uh oh!

noobydp commented Jun 3, 2026

Uh oh!

jasonmadigan left a comment

Uh oh!

noobydp commented Jun 6, 2026

Uh oh!

noobydp commented Jun 21, 2026

Uh oh!

jasonmadigan left a comment

Uh oh!

Uh oh!

noobydp commented Jul 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

noobydp commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Validation

Codex Disclaimer

Uh oh!

noobydp commented Jun 3, 2026

Uh oh!

noobydp commented Jun 3, 2026

Uh oh!

jasonmadigan left a comment

Choose a reason for hiding this comment

direction concern

code-level findings (for when the direction is agreed)

Uh oh!

noobydp commented Jun 6, 2026

Uh oh!

noobydp commented Jun 21, 2026

Uh oh!

jasonmadigan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

noobydp commented Jul 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

noobydp commented May 31, 2026 •

edited

Loading