Repository navigation
fix(ast): Kotlin walker tests read valid Kotlin; tree-sitter-language-pack <1.24 - #939
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…-pack <1.24
The TestKotlin fixture `class A { fun m() {} class B { ... } }` is not valid
Kotlin (declarations need a newline or `;`), so its expected definitions were
the grammar's error recovery, not the walker's contract. Bumping
tree-sitter-language-pack 1.22.1 -> 1.23.1 (#932) changed that recovery and
`unit` failed on test_full_definition_set_in_document_order and
test_object_declaration_emits_no_type_entry (run 42350563661, job
128337627651).
The fixture is now valid Kotlin and a test asserts it parses without error.
Under 1.23.1 a bare `object O { ... }` is an `object_declaration` with a
`type_identifier`, so O and O.o are reported; the test that pinned the 1.22.1
behaviour (no entry for O) now pins the correct one. The pin moves to <1.24
and uv.lock to 1.23.1 (the same entry Dependabot resolved in #932).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018kyZYqc5kJqnz7haDpUyug
17bb517 to
61cbc19
Compare
|
Claude finished @cdeust's task in 1m 46s —— View job PR #939 review (1 of 3, ADR-1119)
No Blockers.
Notes, no action before merge
I could not post this with Verdict: approve |
…anguage-pack <1.24
|
Replies to the review (1 of 3):
Generated by Claude Code |
|
Claude finished @cdeust's task in 1m 0s —— View job PR #939 review (2 of 3, ADR-1119)
No Blockers.
The Kotlin expectations at Notes, no action before merge
I could not post this with Verdict: approve |
|
Reply to review 2 of 3: the rename Generated by Claude Code |
Summary
Unblocks #932 (Dependabot
minor-and-patchgroup). That PR bumpstree-sitter-language-pack1.22.1 → 1.23.1 andunitfails on twoTestKotlintests (run 42350563661, job 128337627651):test_full_definition_set_in_document_order(('O', 'class') != ('o', 'function'),A.mmissing) andtest_object_declaration_emits_no_type_entry.Root cause: the test fixture was not valid Kotlin.
class A { fun m() {} class B { ... } }puts two declarations on one line with no newline or;, so tree-sitter returns an ERROR node and the walker sees whatever error recovery produced. That recovery differs between grammar releases. The tests were pinning the grammar's recovery, not the walker's contract. Separately, the 1.22.1 grammar could not parse a bareobject O { ... }(it came out asinfix_expression+ ERROR). That is the "surprise" one test pinned; 1.23.1 fixes it.Measured locally with both releases (fresh venvs, same walker):
has_error=True;ofunction,A.m,A.deephas_error=True;O,O.o, noA.m,A.deephas_error=True(bareobject) ;ofunctionhas_error=False;O,O.o,A,A.m,B,B.deepChange:
Aon their own lines). The new testtest_fixture_is_valid_kotlinasserts the grammar parses it with no error, so recovery can't creep back into the expectations.test_object_declaration_emits_no_type_entrybecomestest_bare_object_declaration_emits_a_type_entry:OandO.oare reported.<1.24. Theuv.lockentry is set to 1.23.1, the same package block Dependabot resolved in deps: bump the minor-and-patch group with 4 updates #932.uv lock --check --offlinepasses.No production code changes:
_walk_kotlinwas already right for valid source.Refs #932. After this merges, Dependabot's rebase of #932 drops the language-pack part of its group.
Type of change
Test plan
pytest tests_py/core/test_ast_extractor_walkers.py test_ast_extractor_depth.py test_wiki_stub_detector.py test_ast_extractors_multilang.py test_ast_extractor_imports.py test_ast_parser_language_contract.pywith tree-sitter-language-pack 1.23.1: 97 passed.ruff check/ruff format --checkon the touched test file: clean. Repo-wideruff checkreports 8 E501 inscripts/andtests_py/scripts/, and the same 8 show onorigin/mainwithout this change, so the local ruff probably differs from CI's.python scripts/check_craftsmanship.py: OK.uv lock --check --offline: consistent.Not verified locally: the full suite and pyright.
uv synccan't reach download.pytorch.org from this environment, so I installed the locked dependencies without torch/sentence-transformers. In #932's run,typecheckpassed with 1.23.1 in the lock, andunitfailed only on these two tests.Audit notes
Coding-standards compliance
Reviewer checklist
Generated by Claude Code