Skip to content

fix(florence): fix text failure#1582

Merged
leshy merged 1 commit intodevfrom
paul/fix/florence
Mar 19, 2026
Merged

fix(florence): fix text failure#1582
leshy merged 1 commit intodevfrom
paul/fix/florence

Conversation

@paul-nechifor
Copy link
Contributor

Problem

Closes DIM-XXX

Solution

Breaking Changes

How to Test

Contributor License Agreement

  • I have read and approved the CLA.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 16, 2026

Greptile Summary

This PR enhances the Florence2Model.caption() method to accept an optional detail parameter, allowing callers to override the caption detail level (brief/normal/detailed) at call time rather than only at initialization. It also adds a CaptionDetail.from_str() classmethod for convenient string-to-enum resolution.

  • Added CaptionDetail.from_str() classmethod with aliases ("brief", "normal", "detailed", "more_detailed") for resolving detail-level strings to enum members
  • Extended Florence2Model.caption() signature with an optional detail: str | CaptionDetail | None parameter that falls back to the instance default when None
  • The caption_batch() method was not updated with the same detail parameter, creating an API inconsistency between single and batch captioning

Confidence Score: 4/5

  • This PR is safe to merge with low risk — it adds an optional parameter with a sensible default, preserving backward compatibility.
  • The change is small, backward-compatible, and well-structured. The only concern is the API inconsistency between caption() (supports detail override) and caption_batch() (does not), and a minor gap where from_str can raise an unhelpful KeyError on invalid input.
  • No files require special attention; dimos/models/vl/florence.py is the only changed file and the changes are straightforward.

Important Files Changed

Filename Overview
dimos/models/vl/florence.py Adds CaptionDetail.from_str() classmethod and extends caption() with optional detail parameter; caption_batch() not updated with same override capability.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["caption(image, detail=None)"] --> B{detail is None?}
    B -- Yes --> C["Use self._task_prompt\n(instance default)"]
    B -- No --> D{isinstance CaptionDetail?}
    D -- Yes --> E["Use detail.value"]
    D -- No --> F["CaptionDetail.from_str(detail)"]
    F --> G{In _ALIASES?}
    G -- Yes --> H["Return alias match"]
    G -- No --> I["cls[name.upper()]\n(KeyError if invalid)"]
    H --> E
    I --> E
    C --> J["Process image with task_prompt"]
    E --> J
    J --> K["Generate caption"]
Loading

Comments Outside Diff (1)

  1. dimos/models/vl/florence.py, line 138-148 (link)

    caption_batch missing detail override

    caption() now accepts an optional detail parameter, but caption_batch() still hardcodes self._task_prompt. This creates an API inconsistency — users who discover they can override detail level on single captions will expect the same on batches. Consider adding the same detail: str | CaptionDetail | None = None parameter here as well, with the same resolution logic.

Last reviewed commit: 126bb92

@leshy leshy merged commit 00cdcc4 into dev Mar 19, 2026
21 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants