feat: get images tool #53
Conversation
| "image_tools", | ||
| "ImageTools Usage Guide - Only use parameters that the user explicitly requests", | ||
| "prompts/image_tools_prompt.md", | ||
| ) |
There was a problem hiding this comment.
server.py에서 register_prompt(mcp) 주석 해제해야 prompt 확인 가능할 것 같습니다
There was a problem hiding this comment.
add_prompt 시그니처가 궁금해서 확인해봤는데요.
Prompt 인스턴스가 제공되어야 할 것 같은데, 작성하신 코드가 정상 동작하는지 확인 부탁드립니다.
def add_prompt(self, prompt: Prompt) -> Prompt:
"""Add a prompt to the server.
Args:
prompt: A Prompt instance to add
Returns:
The prompt instance that was added to the server.
"""
self._prompt_manager.add_prompt(prompt)
# Send notification if we're in a request context
try:
from fastmcp.server.dependencies import get_context
context = get_context()
context._queue_prompt_list_changed() # type: ignore[private-use]
except RuntimeError:
pass # No context available
return promptThere was a problem hiding this comment.
동해님, 코멘트 주셔서 감사합니다.
결론적으로 프롬프트 쪽 코드는 우선 제거하고 병합하고, 추후 고려해보겠습니다
프롬프트 추가는, Claude 테스트시
input을 "이미지들을 조회해줘"
라고 했을 때
get_images의 인자에 아무것도 없는 {}가 전달되길 예상했으나,
{status: Null, visibility: Null} 이 전달되는 문제를 겪어
프롬프트로 예시를 들어 정확히 의도한 인풋을 전달하고자 했습니다.
다만 프롬프트를 ai로 만들어내는과정에서 해당 코드를 제대로 체크못했던 제 실수로 server.py쪽 호출부 및 잘못된 매개변수의 add_prompt가 올라온부분 인지하였습니다
이후 프롬프트쪽을 모두 지우고 테스트 해본 결과 정상동작하는것을 확인할 수 있었는데
우선 프롬프트를 지우고 올리되 추후 동작결과보장을위해 프롬프트 부분을 고쳐 올리겠습니다~!
tests/tools/test_image_tools.py
Outdated
| else: | ||
| mock_conn.image.images.return_value = [] | ||
|
|
||
| result = ImageTools().get_images(**{filter_name: filter_value}) |
There was a problem hiding this comment.
지금도 좋은데, 개인적으로는 "필터값이 제대로 전달되며 실행되었는지", 그리고 "sdk의 응닶값이 의도한대로 매핑되어 리턴되는지" 정도만 확인해도 될 것 같습니다.
만약 현재 tool에서 응답값으로 다시 한번 필터링 하는 로직이 있다면 해당 테스트 코드가 의미가 있겠지만, 필터링 자체를 구현하는 것은 sdk의 책임이니까요. 편하게 의견 주세요 ~
There was a problem hiding this comment.
음 제가 잘 이해를 못했는데, 너무 다양한 필터 테스트를 한 테스트함수에 작성한 부분을 말씀주신걸까요?
There was a problem hiding this comment.
아뇨 필터링을 테스트 코드 내에서 구현할 필요는 없다는 의견이였습니다. 필터링이 sdk 호출시 인자로 전달되는지, 그리고 sdk의 결과를 그대로 응답하는지 정도만 확인하면 될 것 같습니다
|
image 관련 도구에 대한 prompts를 만드셨느대 어떤 유즈케이스를 고려해서 만들어진건가요?? |
| - `name` is not set (null) | ||
|
|
||
| ### Important Notes | ||
| - If user asks for "public images only", do NOT automatically set `status` to "active" |
There was a problem hiding this comment.
If user asks for "public images only", do NOT automatically set
statusto "active"
해당 내용에 대해 구체적으로 설명해줄 수 있나요?
There was a problem hiding this comment.
@halucinor
승주님, 코멘트 감사합니다.
프롬프트 추가는, Claude 테스트시
input을 "이미지들을 조회해줘"라고 했을 때
get_images의 인자에 아무것도 없는 {}가 전달되길 예상했으나,
{status: Null, visibility: Null} 이 전달되는 문제를 겪어
프롬프트로 예시를 들어 정확히 의도한 인풋을 전달하고자 했습니다.
다만 AI로 프롬프트작성하는 과정에서 제대로 동작하지않는 프롬프트가 작성되었는데
그럼에도불구하고 Claude테스트시 정상동작하여 잘 해결되었다고 잘못판단하였습니다
다만 처음 겪은 문제처럼 프롬프트가없다면 LLM마다 동작이 다를 수 있으므로, 우선 프롬프트를 지우되 추후 추가하는 방향으로 진행코자합니다
There was a problem hiding this comment.
@S0okJu
다경님, 코멘트 감사합니다.
구체적 예시로,
Claude 테스트시 "Public 이미지 조회"라고 인풋을 넣으면
get_images 인자로 {visibility:"public"} 을 예상했으나
{visibility:"public", status:"active"} 라는 인자가 전달되는 문제를 겪어
이를 해결하기위한 가이드 프롬프트를 작성하게 되었습니다
There was a problem hiding this comment.
@S0okJu
제 의견에 대해 곰곰히 생각하여 문서까지 찾아 코멘트 주셔서 감사합니다.
status:active 라는 인자가 전달되면 status가 active인 image가 조회됩니다.
다만 앞선 예시로 "public 이미지를 조회"라는 사용자 요청은 의미는, status의 값과 무관하게 public 인 이미지들이 모두 조회 되어야 된다고 생각합니다.
구체적으로, "public 이미지 조회" 라고 사용자가 질의했을 때 status가 active인것으로 한정하지 않고, status의 값인 importing, uploading, .... 등등 모두 조회되는 것이 질의에 대한 올바른 답변이라는 의도입니다.
실제로 호라이즌의 이미지 조회 필터에서도 visibility를 public으로 설정하면 status와 무관하게 visibility에 한해서만 필터되는 방식으로 동작하여 위처럼 코드를 작성했습니다.
혹시 의견이 다르시면 의견주시면 감사하겠습니다~!
There was a problem hiding this comment.
@jja6312
public이라는 조건을 고정하는 것보다 "기본적으로는 모든 상태를 조회한다" 라고 하는 것은 어떨까요?
만약에 "private image를 줘" 라고 하면 말씀해주신대로 모든 상태를 반환하지 않을 것 같아요.
There was a problem hiding this comment.
@S0okJu
오~! 좋은 의견 감사합니다.
제 생각에도 지금 작성한게 너무 구체적 예시인듯하네요.
지금보다 범용적으로 작성해 보겠습니다~!
halucinor
left a comment
halucinor
left a comment
There was a problem hiding this comment.
코맨트를 읽고 지안님이 생각하신 prompt에 대한 내용을 읽어보니, 해당 내용을 Tool을 어떻게 사용할지 에 대한 맥락을 전달하기 위한 용도로 사용하신 것 같습니다.
다만 이런 내용은 Tool의 docstring으로 작성되어 llm에 맥락(context)으로 전달될 때 더 효과적이라고 생각하는대요, tool에 현재 작성된 내용보다 더 많은 맥락을 담은 docstring을 활용해서 테스트 해보시는건 어떨까요?
| conn = get_openstack_conn() | ||
|
|
||
| # List the servers | ||
| # Build filters for the image query |
There was a problem hiding this comment.
코드를 읽고 직관적으로 이해할 수 있는 내용에 대한 주석은 불필요합니다.
There was a problem hiding this comment.
@halucinor 승주님 좋은 의견 감사합니다.
말씀대로 원하는 결과값을 보기에 docstring으로도 충분해보입니다. 반영하여 작성해보겠습니다!
jja6312
force-pushed
the
feat/glance-get-images-tools
branch
from
b076efe to
d62e35b
Compare
jja6312
force-pushed
the
feat/glance-get-images-tools
branch
from
d62e35b to
ec29077
Compare
|
프롬프트 제거하고 필터값 정상 호출됨을 확인되었습니다 
|
|
@choieastsea 확인부탁드립니다 |
* feat: add get images (#20) * docs: change get_images docstring to sphinx style * refactor: restructure get images testing for better maintainability
* feat: add get images (#20) * docs: change get_images docstring to sphinx style * refactor: restructure get images testing for better maintainability
Key Changes
get images, write a prompt that allows the LLM to select appropriate search filtersRelated Issues
Additional context
statusandvisibilitycan have the following values. (Refer to the API doc image below when implementing Enums later)namevalue only supports exact matches. Horizon also allows searches only with the exact name.If the exact name is not provided, the LLM will attempt to retrieve based on its own judgment. Since this makes the implementation dependent on LLM performance, further discussion is needed on the direction.