feat: auto-detect language in init - #40
Conversation
Manual verification
|
Local test resultsBuilt the Go CLI from this branch on Windows and exercised auto-detect on the in-repo fixtures ( Commands run: Outcome:
Help text reads: |
ar7casper
left a comment
There was a problem hiding this comment.
Lots of value here — let me split the response into two parts.
CI fix (already visible from the failing checks): core/parser_adapter.py:30's bare open(_LANGUAGES_CONFIG) trips the UTF-8 regression scanner that landed in #56 (which you just rebased onto). One-line fix:
from utilities.file_io import read_json
def _load_language_config() -> dict:
return read_json(_LANGUAGES_CONFIG)That's the only thing keeping CI red.
Structural ask: could you drop the MarkFlagRequired("language") removal from this PR?
We've opened #61 to track the validation work needed before we're comfortable flipping -l from required to default-auto. The concern isn't your implementation — it's that the underlying dominance heuristic is byte-for-byte the same as what's on master, and we previously made -l required because real-world reliability wasn't where we wanted it. We'd like to keep -l required for now, expose -l auto as an opt-in users can try, and flip the default once #61's three pieces (E2E fixtures + polyglot regression suite + OSS calibration corpus) give us evidence the algorithm holds up.
That keeps the parts of this PR with the most concrete value for end users — the shared config/languages.json (eliminates Go↔Python drift), the non-git path (real UX win for tarball-style installs), the test infrastructure, the Zig alignment — and just defers the default-flip until #61 is closed.
Concrete asks for this PR:
- Fix the bare
open()(CI green). - Restore
_ = initCmd.MarkFlagRequired("language")ininit.go. - Keep
-l autoworking as an opt-in — your existingif initLanguage == "" || initLanguage == "auto"block stays, just the user has to explicitly write-l autoto trigger it. - Update the
-lhelp text to flagautoas experimental, e.g.Language to analyze: python, javascript, go, c, ruby, php, zig, auto (auto = experimental dominance heuristic; see #61).
Happy to move the validation discussion to #61 if you want to dig into the testing plan there.
Smaller nits for whenever (not blockers):
- The
_LANGUAGES_CONFIGpath resolution does four.parents — fragile if openant-core ever ships standalone or moves.importlib.resourcesreading from inside the package would be more robust. detect_language's docstring still saysReturns: One of: "python", "javascript", "go", "c", "ruby", "php"— missing"zig". Your commit5ac863c3aligned the help text and error messages but the docstring slipped through.
- Make --language optional in `openant init` (default: auto-detect) - Make git repository optional for local paths (use "nogit" placeholder) - Add shared config/languages.json for file extension mappings - Both Go (init.go) and Python (parser_adapter.py) load from the same config file, eliminating duplicated extension lists - Add .worktrees/ to .gitignore for worktree-based workflow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add unit tests for ``detect_language`` covering Python, JS/TS, Go, skip-dir handling, and the empty-directory error path. These tests build synthetic fixtures with ``tmp_path`` and run without the Go toolchain, so they execute in every CI matrix configuration. Add a ``TestInit`` class to ``test_go_cli.py`` that drives the real binary and verifies: - auto-detect on the Python and JS fixtures - auto-detect on synthetic TypeScript and Go trees - explicit ``-l`` flag overrides auto-detection - non-git directory is initialized with a ``nogit`` placeholder SHA - ``--commit`` on a non-git path warns and falls back to ``nogit`` - empty directory fails with a clear "no supported source files" message Item 13 of #16.
Add two tmp_path tests (TS dominant over Python, Python dominant over JS) that exercise the dominance heuristic without relying on skip_dirs to mask competing extensions. Mention Zig in the init -l flag help and in the Go detectLanguage "no supported source files" error so the Go side matches the Python detector (which already lists Zig). The .zig extension is already present in config/languages.json.
Per review: restore MarkFlagRequired("language") so -l remains mandatory
until knostic#61 validation work is complete. Users opt into auto-detection
explicitly with -l auto. Update help text to mark auto as experimental
with a reference to knostic#61.
Also fix parser_adapter.py: use read_json() from utilities.file_io
instead of bare open() (CI regression scanner from knostic#56), keep _JS_PARSER_DIR
alongside _LANGUAGES_CONFIG, and add "zig" to the detect_language docstring.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5f481a2 to
fa79133
Compare
|
Thanks for the detailed review. All four concrete asks are addressed in the latest push:
Also addressed the two nits:
The Rebased onto |
Now that --language is required, all auto-detection tests must opt in with -l auto. Docstring updates to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@ar7casper responded here |
ar7casper
left a comment
There was a problem hiding this comment.
Round-2 review — thanks for the quick turnaround on the four round-1 asks.
Verified each fix in code:
| Round-1 ask | Status |
|---|---|
Fix bare open() (CI green) |
✅ parser_adapter.py:33-34 uses read_json(_LANGUAGES_CONFIG) from utilities.file_io |
Restore MarkFlagRequired("language") |
✅ init.go:58 |
Keep -l auto working as opt-in |
✅ init.go:137 triggers detection on -l auto (or empty) |
Help text marks auto as experimental + points at #61 |
✅ Exactly the wording I'd hoped for |
Bonus: zig added to detect_language docstring |
✅ |
CI went from 6/9 (Python tests red on the bare open()) → 9/9 green. Test suite updated to pass -l auto explicitly in the 7 TestInit cases that exercise auto-detection. No new bugs introduced by the rework.
One small housekeeping nit:
🟢 Low — PR body is stale
The summary still describes round-1's behavior:
Makes
--languageoptional inopenant initby auto-detecting the project language from file extensions in the target path. New users can runopenant init <path>and have it just work.
But the latest commits restored MarkFlagRequired("language") — -l is required again, users now have to explicitly pass -l auto to opt into detection. The body's example list (openant init <url> -l auto) is correct, but the prose summary contradicts the actual behavior.
Suggestion: update the Summary to reflect the current scope (-l required + -l auto opt-in + forward reference to #61 explaining why default-on is gated).
Non-blocking carry-overs (already noted in round-1):
_LANGUAGES_CONFIG = Path(__file__).parent.parent.parent.parent / "config" / ...— the four-.parentwalk still works in the current monorepo layout but breaks if openant-core is ever vendored or pip-installed standalone.importlib.resourcesfrom inside the package would be more robust. Follow-up cleanup.init.go:137doesif initLanguage == "" || initLanguage == "auto"— withMarkFlagRequired, the empty branch is only reachable on an explicit-l "". Consider simplifying toif initLanguage == "auto"and letting the empty case error out naturally. Truly minor.
Verdict: ✅ ready to merge once the body is refreshed (or even as-is if you're fine fixing the body in-flight). Approving via UI.
Covers the seven PRs in this release: - #35: parse --level default → reachable (CLI consistency with scan + Python CLI) - #36: auto-detect dep changes via ~/.openant/venv/.deps-hash - #37: lazy JS parser npm bootstrap on first use - #39: TypeScript/NestJS DI-aware call resolution (constructor + field + functional inject()) - #40: --language auto opt-in for openant init + non-git path support + shared config/languages.json - #49: Express anonymous route handler extraction (route_handler / route_middleware) - #50: --llm-reachability opt-in stage + cross-parser call_graph.json contract Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Makes
--languageoptional inopenant initby auto-detecting the project language from file extensions in the target path. Also drops the assumption that the path is a git repository for local-only mode. New users can runopenant init <path>and have it just work.Addresses item 13 from #16 (does not close the issue).
Test plan
openant init <python-repo>— language detected as Python.openant init <ts-repo>— language detected as TypeScript.openant init <repo> --language go— explicit flag wins.