Skip to content

Parser fixes from #274, #276, #285 (and Julia impl from #319) were silently reverted by merge-conflict resolutions #361

@npkriami18

Description

@npkriami18

Summary

Four already-landed fixes on main have been silently reverted (partially or fully) by two chore: resolve merge conflicts with main commits: 3cf31ec and cb9d25a. These merges accepted the feature-branch side of code_review_graph/parser.py for the ReScript PR (#323) and Julia PR (#319), both of which had forked from db2d2df — before the fixes below landed. The result is 44+ pre-existing test failures on main that directly match the behavior the original PRs set out to fix.

Affected PRs (now effectively reverted on main)

PR Author Merge commit What was lost
#274 @azizur100389 aa627fb CRLF-safe Databricks notebook detection (1 test)
#276 @azizur100389 e6e3144 SHEBANG_INTERPRETER_TO_LANGUAGE, _detect_language_from_shebang(), .ksh extension (10 tests)
#285 @michael-denyer fe383c7 File-attribution fallback at 5 enclosing_func-gated CALLS emission sites + include_file_sources=False in detect_entry_points (7 tests)
#319 @danvinci 75ce2f1 Julia parser implementation (parser.py Julia references dropped 38 → 6; 21 tests)

Reproduction

git checkout main
uv run pytest tests/ --tb=no -q
# 60 failed, 1015 passed, 1 skipped, 6 errors

The 6 errors are a separate issue (TestApplyToolFilter accesses FastMCP._tool_manager, a private API that does not exist in the pinned fastmcp>=2.14.0). Not addressed here.

Root cause (evidence)

Neither conflict emitted markers because the feature branches had not touched those regions at all — only the main side had them — so git merge produced a clean auto-resolution with the wrong side accepted.

Proposed fix

Restore the four fixes via cherry-pick into a single PR: #<FILLED_IN>. Branch: fix/restore-reverted-parser-fixes from main. Result: 60 failed → 16 failed on main (remaining 16 are unrelated Java/PHP/GDScript regressions + 6 _tool_manager errors, all out of scope).

To prevent recurrence

When resolving a merge conflict of parser.py with a long-lived feature branch that forked before recent fixes:

  1. Prefer git merge --strategy=ort -X theirs only for specific hunks the feature branch truly owns — not for the whole file.
  2. Always run uv run pytest tests/test_parser.py tests/test_multilang.py tests/test_notebook.py -q after any merge commit that touches parser.py.
  3. Consider gating parser.py merges behind a mandatory CI job on main.

cc @tirth8205 @azizur100389 @michael-denyer @danvinci

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions