Skip to content

feat: repo-centric chart browsing - Enter on a repo browses its charts (#137) - #150

Merged
hellices merged 3 commits into
mainfrom
feat/helm-repo-browse
Aug 2, 2026
Merged

feat: repo-centric chart browsing - Enter on a repo browses its charts (#137)#150
hellices merged 3 commits into
mainfrom
feat/helm-repo-browse

Conversation

@hellices

@hellices hellices commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Fixes #137

What

The repo screen was management-only: after adding a repository, seeing what it serves meant backing out to the search screen and typing the repoName/ prefix convention by hand.

  • Enter on a repository row dismisses the repo screen with that repo name; the chart picker underneath scopes its search to it (pre-filled repoName/ keyword, searched immediately) — the natural "what does this repo have?" step right after adding one.
  • Esc keeps its meaning: close without touching the keyword underneath.
  • The keyword-first flow is unchanged, and a pending add/update still owns the screen until it finishes (the mutation guard now also rejects a browse while helm repo add/update runs).

HelmRepoScreen becomes ModalScreen[str | None]; the app wires the dismissal to HelmChartSearchScreen.browse_repo, guarded on the picker still being the screen underneath.

Tests

  • test_enter_on_a_repo_row_browses_that_repos_charts — full round trip: repo pick → scoped search ran → results pickable.
  • test_escape_on_repo_screen_keeps_the_search_keyword — Esc never rewrites the keyword.

Docs: helm-operators.md repo-management paragraph.

Full gate (make check) green: 2839 passed.

#137)

The repo screen was management-only: after adding a repository, seeing
what it serves meant backing out and typing the repoName/ prefix by
hand. Enter on a repository row now dismisses the repo screen with that
repo's name, and the chart picker underneath scopes its search to it
(the repoName/ prefix convention, typed for you, searched immediately).
Esc keeps its meaning - close without touching the keyword underneath.
The keyword-first flow is unchanged; a pending add/update still owns
the screen until it finishes.

Fixes #137

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 1, 2026 15:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds repository-centric Helm chart browsing from the repository management modal.

Changes:

  • Returns a selected repository and immediately searches using its prefix.
  • Preserves existing escape and mutation-guard behavior.
  • Adds integration tests and operator documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/korvid/ui/app.py Connects repository selection to chart search.
src/korvid/ui/widgets/helm_chart_search.py Adds repository-scoped browsing.
src/korvid/ui/widgets/helm_repos.py Makes repository rows selectable.
tests/ui/test_helm_actions.py Tests browse and escape flows.
docs/helm-operators.md Documents repository browsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/korvid/ui/widgets/helm_chart_search.py Outdated
Comment thread src/korvid/ui/widgets/helm_repos.py

@my-reviewer-agent my-reviewer-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이슈 #137(레포 중심 차트 브라우징) 구현 리뷰 — 신규 PR.

요약: HelmRepoScreenModalScreen[str | None]로 바뀌어 레포 행에서 Enter 시 레포 이름으로 dismiss하고, 아래의 HelmChartSearchScreen.browse_repo()repo/ 프리픽스를 입력창에 채워 즉시 검색합니다. 설계가 깔끔합니다:

  • stale-selection 가드: _pickedself.screen is browse_in을 재확인해 픽커가 이미 사라진 경우 아무것도 하지 않음 — 기존 리뷰에서 요구해온 패턴 그대로.
  • mutation 소유권: on_option_list_option_selected_mutating 중에는 선택을 거부 (helm repo add/update 서브프로세스가 화면을 소유하는 기존 계약 유지). Esc(action_close)와 동일한 게이트.
  • 인덱스 안전: 0 <= option_index < len(self._repos) 경계 체크 + _refresh_list에서 clear_options()self._repos 갱신이 항상 짝으로 이루어져 표시 행과 매핑이 일치. HelmError 경로에서는 둘 다 갱신하지 않아 stale하지만 서로 일관된 쌍으로 남음.
  • 검색 경합: browse_repo가 기존 _start_search(_search_seq 세대 가드, #107에서 승인)를 재사용.
  • 테스트: Enter→스코프 검색 실행 + 키워드 반영 + 결과 픽 가능까지 확인하는 테스트와, Esc로 닫으면 기존 키워드를 덮어쓰지 않는 회귀 테스트 모두 실질적 단언.

문서(docs/helm-operators.md)와 상태바 힌트도 함께 갱신됨. 발견된 결함 없음.

APPROVE

…n guard

Review round 1 on #150:

- helm search repo substring-matches, so browsing 'stable' also
  surfaced 'my-stable/...' charts. browse_repo now passes a repo scope
  through _start_search and the results are filtered to the exact
  'repo/' name prefix; manual re-searches stay unfiltered. RED test:
  test_repo_browse_filters_to_the_exact_repo_prefix.

- the mutation guard on row selection had no test teeth: a gated
  repo_add test now proves Enter mid-mutation keeps the screen open
  and the add completes (test_repo_pick_is_rejected_while_a_mutation_is_pending).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@my-reviewer-agent my-reviewer-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

재리뷰 — 신규 커밋 1건 (라운드 1 후속).

  • helm search repo가 substring 매칭이라 stable 브라우즈 시 my-stable/...까지 노출되던 문제를 repo_scope 파라미터로 해결: browse_repo → _start_search(repo_scope=repo) → seq 가드 통과 후 hit.name.startswith(f"{repo}/") 정확한 접두사 필터. 수동 재검색은 scope=None이라 필터 없이 동작 — 의도대로. RED 테스트(my-stable/sneaky fixture)로 검증됨.
  • 뮤테이션 가드에 테스트가 없던 점도 해결: gate로 repo_add를 붙잡아둔 상태에서 Enter가 화면을 dismiss하지 않고(워커/서브프로세스 보존) add가 끝까지 완료되는 것을 단언.

지적사항 없음. LGTM.

APPROVE

@hellices

hellices commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Status: review loop complete (round 1 findings fixed with RED-verified tests, round 2 fully clean — no new or suppressed comments, all threads resolved) and the full local gate is green (2841 passed). The required test checks are stuck QUEUED because the korvid-runners self-hosted runner went offline mid-run ("The self-hosted runner lost communication with the server") and has not picked jobs up for over an hour. Once the runner is back and checks are green, this is ready to gh pr merge 150 --squash.

@hellices
hellices merged commit 6e0d195 into main Aug 2, 2026
9 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.

Helm UX: repo-centric chart browsing (pick a repo → browse its charts)

2 participants