Skip to content

fix(ci): restore green CI — pin lint rules, repair two stale test assumptions - #51

Merged
lexasub merged 1 commit into
mainfrom
fix/ci-deterministic-lint
Aug 1, 2026
Merged

fix(ci): restore green CI — pin lint rules, repair two stale test assumptions#51
lexasub merged 1 commit into
mainfrom
fix/ci-deterministic-lint

Conversation

@r0h1tb

@r0h1tb r0h1tb commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

CI has been red since 2026-07-27. Because ruff check runs before pytest in the workflow, the Run tests step has been skipped ever since — I confirmed this in the job step list. The suite has not executed in CI for a week, so nothing merged in that window was actually tested.

Three independent causes, none of them a code regression.

1. Lint was non-deterministic across ruff releases

[tool.ruff] set only target-version and line-length — never select. Ruff therefore applied whatever its current default rule set happened to be, and pyproject.toml pins ruff only as "ruff", so CI installs the latest on every run.

A newer ruff (0.16.x) widened that default, and the tree went from clean to 496 findings with no code change — 321 of them UP045 alone.

Evidence that it's version drift rather than a commit: running today's ruff against 16ae8bd, the last commit whose CI run was green, also reports 496.

commit ruff 0.16.1 findings
16ae8bd (CI green, 2026-07-06) 496
cd75313 (main) 496

Fix: select = ["E4", "E7", "E9", "F"] — the rules the tree is actually clean under, and what CI was effectively enforcing when it last passed. A future ruff release can no longer change what CI means.

If you'd rather adopt the wider modern rule set, that's a reasonable call too — but it's ~496 findings and belongs in its own PR, not one that's blocking everything else.

2. mcp 2.0 renamed FastMCP

mcp>=1.0 is unpinned, so fresh installs resolve to 2.x, where FastMCP became MCPServer and moved out of mcp.server.fastmcp. ast_rag/mcp/server.py failed at import, taking both tests in test_update_project_dry_run.py with it.

Imports the new name with a fallback to the old, so both 1.x and 2.x work. I verified MCPServer is API-compatible with the .tool() decorator usage in that module before aliasing.

3. A stale TSX assertion

test_supported_extensions_grouped_by_language still asserted typescript == [".ts", ".tsx"], but cd75313 deliberately moved .tsx onto a dedicated TSX grammar and added .jsx to it. The code is right; the assertion was left behind. It now pins the intended grouping, .jsx included.

Verification

All three CI steps, run locally on this branch:

ruff check ast_rag/        All checks passed!
pytest tests/              176 passed, 1 skipped, 2 xfailed
ruff format --check        75 files already formatted

On main, the same three give 496 lint errors and 3 failed, 174 passed.

No production behaviour changes — CI configuration plus two test/import corrections.

Note

This is deliberately independent of #50 and branches from main, so it can merge on its own. #50 will stay red until this lands, since it inherits the same broken lint step.

…umptions

CI has been failing since 2026-07-27 and, because `ruff check` runs before
`pytest`, the "Run tests" step has been *skipped* ever since. The suite has not
actually executed in CI for a week.

Three independent causes:

1. `[tool.ruff]` set only target-version and line-length, never `select`, so
   ruff applied whatever its current default rule set was. A newer ruff
   (0.16.x) widened that default and the tree suddenly reported 496 findings
   with no code change -- 321 of them UP045 alone. Verified by running today's
   ruff against 16ae8bd, the last commit whose CI run was green: it also
   reports 496. Pinning `select = ["E4","E7","E9","F"]` -- the rules the tree
   is clean under -- makes lint deterministic across ruff releases.

2. `mcp` 2.0 renamed `FastMCP` to `MCPServer` and moved it out of
   `mcp.server.fastmcp`, so `ast_rag/mcp/server.py` failed to import and took
   both tests in test_update_project_dry_run.py with it. `mcp>=1.0` is
   unpinned, so fresh installs get 2.x. Imports the new name with a fallback
   to the old one; `MCPServer` is API-compatible for the `.tool()` decorator
   usage here.

3. test_supported_extensions_grouped_by_language still asserted
   `typescript == [".ts", ".tsx"]`, but cd75313 deliberately moved .tsx onto a
   dedicated TSX grammar and added .jsx to it. The code is correct and the
   assertion was stale; it now pins the intended grouping, including .jsx.

Local run of all three CI steps on this branch:

  ruff check ast_rag/           All checks passed!
  pytest tests/                 176 passed, 1 skipped, 2 xfailed
  ruff format --check           75 files already formatted

On main the same three steps give 496 lint errors, and 3 failed / 174 passed.

No production behaviour changes; this is CI configuration plus two test/import
corrections.
@r0h1tb
r0h1tb requested a review from lexasub as a code owner August 1, 2026 18:48
@github-project-automation github-project-automation Bot moved this to Backlog in raged kanban Aug 1, 2026
lexasub pushed a commit that referenced this pull request Aug 1, 2026
Reports the four things the issue asks for -- node counts by kind, edge counts
by type, language distribution and file count -- plus --json for scripting.

Edges are grouped by the `kind` property rather than by relationship type,
because batch_upsert_edges writes everything as a generic :EDGE and carries the
semantic type as a property. Grouping by type(r) would report a single 'EDGE'
bucket. A test pins that so the query can't regress to type(r).

The CurrentVersion bookkeeping node is excluded from node counts; it is the
graph's version pointer, not code.

Verified against a 103-file Spring Boot index:

  Files indexed: 103
  Nodes (921 total): Field 414, Method 368, Class 100, Constructor 27, ...
  Edges (951 total): CONTAINS_FIELD 409, CONTAINS_METHOD 392, CALLS 150
  Languages: java 921 (100%)

Suite: 3 failed, 179 passed (baseline on main: 3 failed, 174 passed) -- same
three pre-existing failures, fixed separately in #51.
lexasub pushed a commit that referenced this pull request Aug 1, 2026
Uses the existing rich dependency rather than adding tqdm.

Parsing previously used console.status(), an indeterminate spinner. It now
reports a bar with count, elapsed and ETA alongside the current file name.

The bigger gap was embeddings. build_embeddings() ran behind a single
'Building embeddings...' spinner with no output until it finished. On a first
run that also downloads the model this is many minutes of apparent hang -- I
had to query Qdrant directly to confirm the process was alive. It now takes an
optional progress_callback(done, total), invoked after each batch, and the CLI
renders it as a bar. Default is None, so the API is unchanged for existing
callers.

The callback also fires once with (0, 0) when there is nothing embeddable, so
callers always get a terminal update instead of a bar that never resolves.

Bars are transient, so they clear on completion and leave the existing summary
lines as the only residue.

Verified by re-indexing a 105-file Spring Boot project end to end.

Suite: 3 failed, 178 passed (baseline on main: 3 failed, 174 passed) -- same
three pre-existing failures, fixed separately in #51.
lexasub pushed a commit that referenced this pull request Aug 1, 2026
edge_extractor.py defines four module-level functions twice. Python keeps the
last definition, so the first copy of each has never executed:

  _node_text                 660-662     shadowed by 1282-1283
  _find_enclosing_type       671-680     shadowed by 1286-1294
  _find_enclosing_callable   683-692     shadowed by 1297-1305
  _add_type_relation_edges   695-1279    shadowed by 1308-1371

The last of those is the interesting one. It spans 585 lines because a block
was indented one level too deep during the extract-methods refactor, nesting a
second copy of ten EdgeExtractor methods inside it as local functions --
including a second _extract_call_edges carrying the same `for n in []` defect
as the real one. None of it is reachable.

This deletes the shadowed copies, which is a no-op by construction: the
interpreter was already discarding them. Verified by extracting each live
callable's source before and after -- all five are byte-identical.

The surviving copies had lost their docstrings when they were duplicated, so
those are carried back over. That is the only content change.

edge_extractor.py: 1371 -> 759 lines. EdgeExtractor itself (lines 31-657) is
untouched.

Re: #10 -- the split this issue asks for has already happened; extract_edges
delegates to _extract_containment_edges / _extract_import_edges /
_extract_call_edges and six more. The issue also points at ast_rag/ast_parser.py,
which no longer exists. What remained was this debris from that refactor.

Suite unchanged from main: 3 failed, 174 passed, 2 xfailed (same three
pre-existing failures, addressed separately in #51).
@lexasub
lexasub merged commit fb16312 into main Aug 1, 2026
1 check passed
@lexasub
lexasub deleted the fix/ci-deterministic-lint branch August 1, 2026 23:19
@github-project-automation github-project-automation Bot moved this from Backlog to Done in raged kanban Aug 1, 2026
lexasub pushed a commit that referenced this pull request Aug 1, 2026
Adds tree-sitter-go and a GO_QUERIES module covering structs, interfaces,
functions, methods, struct fields, imports and calls.

Go models types as `type_declaration -> type_spec` with the concrete shape on
the `type` field, so structs and interfaces are matched on the type_spec
rather than on a dedicated node. Methods carry a `receiver` and are a separate
node type (method_declaration) from plain functions, so Area() lands as METHOD
while describe()/main() land as FUNCTION.

The calls query handles both shapes Go uses -- a bare identifier (describe())
and a selector (fmt.Println()) -- capturing the method name for the latter.
Query names reuse the existing generic mapping in node_extractor, so no
per-language dispatch was needed.

Verified on a sample with an interface, struct, method, two functions and both
call shapes:

  INTERFACE Shape / STRUCT Rect / FUNCTION describe, main / METHOD Area
  FIELD W, H / IMPORTS 1

Call edges additionally require the _extract_call_edges fix from #50; with that
applied locally this sample yields exactly the two intra-file edges
(describe -> Area, main -> describe) and correctly excludes external calls such
as fmt.Println. This PR does not include that fix.

test_unsupported_language.py used .go as its example of an unsupported
extension, which is no longer true -- switched to .rb.

Suite: 3 failed, 188 passed (baseline on main: 3 failed, 174 passed) -- same
three pre-existing failures, fixed separately in #51.
r0h1tb added a commit that referenced this pull request Aug 2, 2026
…table

Edge extraction resolved references against name_to_id built from a single
file's nodes, so any reference to a symbol defined elsewhere was dropped. Only
same-file calls ever linked.

Measured against ground truth from parsers independent of tree-sitter (Python's
stdlib ast, and javalang for a Java project), the unreachable share was 83% on
this repo and 95% on a Spring Boot service -- in layered code essentially every
interesting call crosses a file.

Indexing is now two-phase: collect nodes from every file and build a project
wide name -> id map, then resolve edges against it. extract_edges takes an
optional global_symbols map; local definitions are applied last so a file-local
symbol always shadows a same-named symbol from another file.

Measured on this repo, before -> after:

  CALLS edges           498 -> 1412
  of which cross-file     0 ->  914
  total edges          2387 -> 3782

Retrieval quality over the 15 most-called symbols, scored against the stdlib
ast oracle across all call relationships (not just same-file):

  precision 0.99, recall 0.95, F1 0.97

Precision holding at 0.99 is the load-bearing result: name-based global
resolution could have produced false positives across same-named symbols, and
on this codebase it does not.

Suite: 3 failed, 188 passed (baseline: 3 failed, 174 passed) -- same three
pre-existing failures, fixed separately in #51.

Refs #35
r0h1tb added a commit that referenced this pull request Aug 2, 2026
…table (#56)

* feat(index): resolve cross-file references via a project-wide symbol table

Edge extraction resolved references against name_to_id built from a single
file's nodes, so any reference to a symbol defined elsewhere was dropped. Only
same-file calls ever linked.

Measured against ground truth from parsers independent of tree-sitter (Python's
stdlib ast, and javalang for a Java project), the unreachable share was 83% on
this repo and 95% on a Spring Boot service -- in layered code essentially every
interesting call crosses a file.

Indexing is now two-phase: collect nodes from every file and build a project
wide name -> id map, then resolve edges against it. extract_edges takes an
optional global_symbols map; local definitions are applied last so a file-local
symbol always shadows a same-named symbol from another file.

Measured on this repo, before -> after:

  CALLS edges           498 -> 1412
  of which cross-file     0 ->  914
  total edges          2387 -> 3782

Retrieval quality over the 15 most-called symbols, scored against the stdlib
ast oracle across all call relationships (not just same-file):

  precision 0.99, recall 0.95, F1 0.97

Precision holding at 0.99 is the load-bearing result: name-based global
resolution could have produced false positives across same-named symbols, and
on this codebase it does not.

Suite: 3 failed, 188 passed (baseline: 3 failed, 174 passed) -- same three
pre-existing failures, fixed separately in #51.

Refs #35

* fix(api): bind call_kinds on the reference and impact queries

UAT against a live index found 'ast-rag refs' and 'ast-rag symbol-impact'
failing with:

  Neo.ClientError.Statement.ParameterMissing
  Expected parameter(s): call_kinds

Regression from the call-traversal rewrite: two queries were changed to filter
on $call_kinds, but their session.run() calls were never given the parameter.
Neo4j only reports this at execution time, so nothing caught it -- the unit
tests never reach these branches without a populated graph.

Binds the parameter at both sites. After the fix, 'refs ParserManager' returns
its references and 'symbol-impact ParserManager' reports 34 references and
42 callers.

Adds a static checker over the API and repository layers: for every
session.run(<var>, **kwargs) it resolves <var> back to its query text and
asserts each $parameter is bound. Reverting the fix makes it fail, which is
the property the first version of this test lacked -- a runtime test could not
reach the broken branch and passed either way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants