feat(cli): add --language flag and prompt to openkb init - #48
Conversation
44c8267 to
a2173c3
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
|
A few smaller follow-ups from the same review pass that didn't make the main report — none blocking, but worth considering before merge: Empty Adding a 3rd prompt to
|
966a146 to
b212020
Compare
Address review on #(this PR): - Strip/validate --language values via _coerce_language: blank/whitespace falls back to DEFAULT_CONFIG['language'] instead of persisting an empty string that would render as a broken "Write all content in language." prompt in compiler.py and query.py. - Reject >50-char or control-char --language values so external callers cannot smuggle instructions into the LLM system prompt via the flag. - Gate the language prompt with _stdin_is_tty() (mirrors _stream_to_tty from VectifyAI#45) so existing `printf '\n\n' | openkb init` automation keeps working instead of aborting on the newly-added third prompt. - Revert test inputs to "\n\n" as a regression guard for the non-TTY path, mock _stdin_is_tty=True for the prompt-exercising test, and add coverage for empty/whitespace/unsafe --language values.
b212020 to
57d8ab6
Compare
|
Addressed all three in 57d8ab6:
|
1. **webbrowser.open return value silently dropped on headless boxes** The command printed "Opening GitHub in your browser..." and exited 0 even when webbrowser.open returned False (no GUI, no $BROWSER, CI runner, container without DISPLAY). Now we capture the return value, surface "no browser available — copy the URL above" to stderr, and only print "Opened GitHub in your browser." after a confirmed True return. 2. **_openkb_version was the third copy of the same logic** openkb/__init__.py exports __version__ via importlib.metadata with fallback "0.0.0+unknown"; openkb/agent/chat.py wraps it as _openkb_version(); cli.py was re-implementing it with fallback "unknown" — already drifted. Replaced with the same one-liner used in chat.py: `from openkb import __version__; return __version__`. Now all three call sites agree. 3. **type prompt hung in non-TTY contexts (regression of PR #48)** PR #48 introduced _stdin_is_tty() specifically because adding a prompt without a TTY check broke automation pipelines. PR #53's second prompt (asking for feedback type) repeated the same pattern. Now it skips the prompt when stdin isn't a TTY and falls through to `--type other`. Pipes / CI work without flags now: echo "..." | openkb feedback "msg" # works, type=other, no label 4 new regression tests: - test_feedback_skips_type_prompt_when_stdin_is_not_a_tty - test_feedback_warns_when_webbrowser_open_returns_false - test_feedback_confirms_when_webbrowser_open_succeeds - test_openkb_version_helper_matches_package_version 331 tests pass (327 prior + 4 new).
Mirrors the --language pattern from VectifyAI#48 to close the remaining asymmetry where `openkb init` was only partially non-interactive. - Add `--model/-m MODEL` flag that skips the interactive model prompt when set, persisting the LiteLLM "provider/model" string straight to .openkb/config.yaml. - Gate the model `click.prompt` on `_stdin_is_tty()` so piped/redirected callers fall back to the default model without a click prompt failure on EOF. - Add `_coerce_model` validation (max 100 chars, no control chars), matching `_coerce_language` so embedded newlines can't corrupt logged output or config.yaml. Now `LLM_API_KEY=... openkb init -m anthropic/claude-sonnet-4-6 -l ko` is fully non-interactive (api_key prompt remains for security).
Mirrors the --language pattern from #48 to close the remaining asymmetry where `openkb init` was only partially non-interactive. - Add `--model/-m MODEL` flag that skips the interactive model prompt when set, persisting the LiteLLM "provider/model" string straight to .openkb/config.yaml. - Gate the model `click.prompt` on `_stdin_is_tty()` so piped/redirected callers fall back to the default model without a click prompt failure on EOF. - Add `_coerce_model` validation (max 100 chars, no control chars), matching `_coerce_language` so embedded newlines can't corrupt logged output or config.yaml. Now `LLM_API_KEY=... openkb init -m anthropic/claude-sonnet-4-6 -l ko` is fully non-interactive (api_key prompt remains for security).
2 participants
Summary
--language/-l LANGflag toopenkb initso the wiki output language can be set non-interactively (e.g.openkb init --language ko).Wiki language (enter for default en)prompt that runs after the LLM API-key prompt when the flag isomitted; pressing enter accepts the configured default.
.openkb/config.yamlaslanguage:instead of hard-codingDEFAULT_CONFIG["language"].Why
Previously
openkb initalways wrote the default English language toconfig.yaml, so non-English users had to hand-editthe file right after initialization. Exposing it at init time — both via flag (for scripts / CI) and via prompt (for the
interactive walkthrough) — removes that manual step.
Test plan
tests/test_cli.py— 7/7 pass:test_init_defaults_language_to_en— no flag, default kept.test_init_language_flag_sets_config/test_init_language_short_flag—--language/-lskip the prompt andwrite the value.
test_init_language_prompt_accepts_input— interactive path writes user input.test_init_creates_structure/test_init_schema_content/test_init_already_existsupdated to feedprompt input.
pytest) — 224 passed.openkb init --language koandopenkb init -l Koreanin temp dirs →.openkb/config.yamlreflects the value;openkb init --helpshows the flag.