Skip to content

fix(export): preserve Hermes vision capabilities - #3151

Closed
Ingwannu wants to merge 1 commit into
devfrom
ingw/fix-hermes-export-capabilities
Closed

fix(export): preserve Hermes vision capabilities#3151
Ingwannu wants to merge 1 commit into
devfrom
ingw/fix-hermes-export-capabilities

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Replace the Hermes string-only model array with the metadata map Hermes uses for custom-provider capability lookup.
  • Emit supports_vision true only for rows whose exported catalog modalities include image, false for an explicit non-empty modality list without image, and no flag when modalities are unknown.
  • Preserve stable selector ordering, existing admission references, YAML round-tripping, CLI and management parity, and the no-guessed-video boundary.
  • Document the export contract and decision tradeoffs.

Closes #3146

Verification

  • Focused client/export, CLI, management, writer, and lifecycle suites: 198 passed, 0 failed.
  • TypeScript: bun ./node_modules/typescript/bin/tsc --noEmit passed.
  • git diff --check passed.
  • Changed-graph run selected 181 files: 3241 passed and 1 skipped; it also reported existing unrelated storage-policy and TOML-parser failures. The four named failing files reproduced identically on a clean origin/dev worktree. The PR stays Draft until exact-head CI resolves the remaining runner failure/error.
  • Protected local OpenCodex, Codex, and Paseo config file modes, sizes, and SHA-256 values were unchanged after testing.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. This patch changes no credential, auth, or destination logic and preserves the existing env-reference-only export.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@Ingwannu

Ingwannu commented Sep 1, 2026

Copy link
Copy Markdown
Owner Author

Exact-head hosted CI update for 5ced04d:

I am keeping #3151 Draft and will not rerun to hide a timing-dependent baseline failure. Once the dev fixture fix lands and the exact head is current, the matrix should be rerun before Ready transition. The requested @lidge-jun review remains in place.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 68 / 80

설명

이 PR은 이슈 #3146을 고칩니다. Hermes export가 모델 목록을 문자열 배열로만 내보내면, Hermes 동적 라우터가 custom provider에서 vision을 못 찾아 텍스트 전용으로 가정합니다. 패치는 Hermes가 custom-provider capability lookup에 쓰는 metadata map으로 바꾸고, supports_vision 을 modalities에 image가 있으면 true, image 없는 명시적 modality면 false, 모르면 플래그 생략으로 냅니다. 선택자 순서·admission·YAML round-trip·CLI/management 패리티·video 추측 금지는 유지합니다.

지금 HEAD src/clients/config-export.tsHermesProviderBlock.modelsstring[] 이고 buildHermesClientConfig 가 그 형태를 만듭니다. 이슈 본문의 flat YAML과 일치합니다. #3047/#3048 Aside, #3082 Hermes mark 등은 export 클라이언트 표면을 키웠지만, Hermes models 직렬화 shape는 그대로였습니다. 베이스 dev, ingw/fix-hermes-export-capabilities, MERGEABLE, +97/−13 입니다. 파일은 config-export, structure, 여러 export 테스트입니다. types/config 분할 무관합니다. Draft로 두었다가 CI를 기다린다고 본문이 적습니다.

점수는 68입니다. vision 모델(Gemini/Luna/Grok 등)을 Hermes로 쓰는 사용자에게 직접 보이는 기능 구멍이고, #3146과 1:1입니다. remote hub보다는 클라이언트 export 축입니다.

라인 src/clients/config-export.ts HermesProviderBlock - models 타입이 string[]에서 map/객체로 바뀝니다. 기존 YAML을 읽는 사용자·테스트 픽스처 마이그레이션이 있는지 보세요
경로 supports_vision 결정 - modalities에 image 포함 여부만 봅니다. #3105 catalog vision-sidecar modalities 와 같은 정의를 쓰는지 확인하십시오
경로 video - 본문이 no-guessed-video를 강조합니다. supports_vision만 넣고 video 플래그를 추측하지 않아야 합니다
경로 issue #3146 - Closes #3146 이 걸려 있습니다. 머지 시 이슈를 닫으십시오
경로 Draft/CI - changed-graph의 무관 storage/TOML 실패를 이유로 Draft면, exact-head CI만 보고 Ready로 올리면 됩니다

메인테이너의 판단이 필요한 지점

  • Hermes 쪽 스키마가 map을 공식 지원하는지(본문 주장) 문서 링크가 충분한지
  • 기존 string[] export를 읽은 Hermes 설치를 덮어쓸 때 동작이 깨지지 않는지

너의 추천
CI 초록이면 Ready 후 머지하고 #3146을 닫습니다. export 계약 문서(structure/09)가 같이 가므로 changelog에 Hermes vision 한 줄을 남기면 좋습니다. 라벨은 바꾸지 않습니다.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun

Copy link
Copy Markdown
Owner

Landed via maintainer carry #3178, merged to dev as 51c4917. Your commit was cherry-picked unchanged with author credit preserved — git show --stat on the carry reports the same 7 files, +97/-13.

Why a carry rather than a merge of this branch: the head here sat 105 commits behind dev, past the repository's 10-commit freshness boundary, so the CI run no longer described what would land, and a maintainer cannot push a rebase to a contributor branch. The rebase itself was clean, one auto-merge in structure/09_client-integrations.md.

Your read of the red CI was right and worth recording: the ci failure was only a rollup for platform-macos, whose single failing test was server local API auth > websocket passthrough refreshes pool auth for each response.create turn — a known macOS flake in websocket auth, unrelated to the Hermes export surface. Thanks for the patch.

@lidge-jun lidge-jun closed this Sep 1, 2026
@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 1, 2026
@lidge-jun
lidge-jun deleted the ingw/fix-hermes-export-capabilities branch September 2, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants