Skip to content

(feat):added up the .robot and .resource file extension in CRG - #967

Open
Bakul2006 wants to merge 8 commits into
tirth8205:stagingfrom
Bakul2006:robotextension
Open

Bakul2006 wants to merge 8 commits into
tirth8205:stagingfrom
Bakul2006:robotextension

Conversation

@Bakul2006

Copy link
Copy Markdown
Contributor

Pull Request

Linked issue

Closes #963

What & why

This PR adds support for .robot Robot Framework test files in the code-review graph.

Previously, Robot Framework files were not recognized by the file extension handling, which meant .robot test files were skipped during processing.

This change adds the .robot extension to the supported test file extensions so that Robot Framework test files can be included in the existing parsing and code-review graph workflow.

How it was tested

The test suite was run successfully:

uv run pytest tests/ --tb=short -q

@github-actions

github-actions Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

code-review-graph review

Overall risk: 0.40 (MEDIUM) — 13 changed function(s)/class(es), 0 affected flow(s), 5 test gap(s) (3 with no tested caller found, 2 reached only through a caller)

Risk-scored changes

Risk Level Symbol Location Tested
0.40 medium code_review_graph/parser.py::CodeParser.qualified code_review_graph/parser.py:4519 no
0.40 medium code_review_graph/parser.py::CodeParser.normalize_keyword code_review_graph/parser.py:4522 no
0.40 medium code_review_graph/parser.py::CodeParser.finish code_review_graph/parser.py:4525 no
0.25 low tests/test_parser.py::TestCodeParser.test_robot_control_flow_and_variable_assignment tests/test_parser.py:91 (test)
0.25 low tests/test_parser.py::TestCodeParser.test_robot_resource_import_is_found_by_importers_query tests/test_parser.py:187 (test)
0.15 low code_review_graph/parser.py::CodeParser code_review_graph/parser.py:2944 yes
0.10 low code_review_graph/parser.py::CodeParser._extract_bytes code_review_graph/parser.py:3193 indirect
0.10 low code_review_graph/parser.py::CodeParser._parse_robot code_review_graph/parser.py:4494 indirect
0.05 low tests/test_parser.py::TestCodeParser tests/test_parser.py:13 (test)
0.05 low tests/test_parser.py::TestCodeParser.test_parse_robot_and_resource_files tests/test_parser.py:26 (test)

Test gaps

  • code_review_graph/parser.py::CodeParser.qualified (code_review_graph/parser.py:4519)
  • code_review_graph/parser.py::CodeParser.normalize_keyword (code_review_graph/parser.py:4522)
  • code_review_graph/parser.py::CodeParser.finish (code_review_graph/parser.py:4525)

Reached only through a caller

No test names these directly, but a tested caller reaches them along the call path shown. That is a path in the call graph, not a record of execution -- the caller's tests may never take this branch. Treat it as where to look, not as coverage: these count as gaps and are scored as untested.

  • code_review_graph/parser.py::CodeParser._extract_bytes (code_review_graph/parser.py:3193) — via code_review_graph/parser.py::CodeParser.parse_bytes, 1 hop(s)
  • code_review_graph/parser.py::CodeParser._parse_robot (code_review_graph/parser.py:4494) — via code_review_graph/parser.py::CodeParser.parse_bytes, 2 hop(s)

Token savings: this graph-backed report used ~238,916 fewer tokens (~99%) than reading every changed file in full (estimated, chars/4 approximation).


Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

@tirth8205

Copy link
Copy Markdown
Owner

Outside this sweep's mission of demonstrated coding-session token savings: this adds a new Robot Framework parser. Bare resource/keyword targets and incomplete setup, teardown and control-flow handling do not yet establish reliable compact cross-file context.

@tirth8205
tirth8205 changed the base branch from main to staging September 15, 2026 13:10
@tirth8205

tirth8205 commented Sep 15, 2026 •

Copy link
Copy Markdown
Owner

This no longer merges into staging. Conflicts in README.md.

git fetch origin && git merge origin/staging
# resolve, then
uv run pytest tests/ -q
uv run ruff check code_review_graph/
uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional
git push

When resolving:

  • Rebase PR onto current origin/staging and resolve README.md conflicts (two hunks: the 'Language coverage' paragraph around line 142, and the feature table 'Language and notebook support' row around line 279); staging rewrote both sections to point at a single Language coverage anchor instead of list...
  • After rebase, re-run the full gate set (pytest on 3.13, ruff, mypy) and the edge-case coverage review for the Robot Framework parser change in code_review_graph/parser.py and tests/test_parser.py.

A large integration branch landed on staging today, which is why this drifted.

PRs now target staging, not main. Yours was retargeted already, so nothing to do there.

@tirth8205 tirth8205 added the needs-rebase Branch no longer merges into staging label Sep 15, 2026
@Bakul2006

Copy link
Copy Markdown
Contributor Author

@tirth8205 solved up the readme.md conflicts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Parser correctness gaps and duplicate or stale README documentation remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds structural Robot Framework support for .robot and .resource files in the parser and code-review graph.

Changes:

  • Maps both extensions to the Robot fallback parser.
  • Extracts Robot sections, tests, keywords, calls, and imports.
  • Adds parser tests and documentation updates.
File summaries
File Summary Review findings
tests/test_parser.py Adds Robot parsing coverage. None.
README.md Documents Robot Framework support. Nit (3 votes): consolidate duplicate parser-support text and feature tables.
CONTRIBUTING.md Documents Robot parser maintenance. None.
code_review_graph/parser.py Adds Robot extension handling and structural parsing. Moderate: support pipe-delimited rows (3 votes), resolve relative resources (2 votes), emit TESTED_BY mirrors (3 votes), and normalize keyword matching (1 vote).
Review details

Suppressed comments (1)

code_review_graph/parser.py:4057

  • Robot keyword matching is case-insensitive and treats spaces and underscores equivalently, but this stores the call target verbatim and the later resolver matches node names exactly. A call such as prepare_environment or prepare environment will remain unresolved instead of linking to Prepare environment, degrading callers/callees and test coverage; preserve a canonical Robot keyword key or add a Robot-specific resolver.
            keyword = cells[0]
            if keyword == "...":
                continue
            edges.append(EdgeInfo(
  • Files reviewed: 4/4 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread code_review_graph/parser.py Outdated
Comment thread code_review_graph/parser.py Outdated
Comment thread code_review_graph/parser.py
Comment thread README.md Outdated
Comment thread README.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@tirth8205

Copy link
Copy Markdown
Owner

The merge is clean now and the gates pass: your head 4a7167e merged into staging 9c13f4c gives 3336 passed, 9 skipped, 2 xpassed, ruff clean, mypy clean, and all 16 CI checks green.

The README is not resolved though. Both sides were kept instead of one.

On the merged tree:

  • The feature table has 55 rows where staging has 31, and 20 of them appear twice: Incremental updates, Framework-aware PHP parsing, Blast-radius analysis, Auto-update hooks, Semantic search, Interactive visualisation, Surprise scoring, Knowledge gap analysis, Suggested questions, Edge confidence, Graph traversal, Export formats, Estimated context savings, Memory loop, Community auto-split, Execution flows, Community detection, Architecture overview, Risk-scored reviews, Custom languages, Token benchmarking. Reproduce with grep -oE '^\| \*\*[^*]+\*\*' README.md | sort | uniq -d.
  • README lines 142 and 143 are two paragraphs saying the same thing. 142 is your pre-merge wording, 143 is staging's.

What to do: drop your side in both places and re-apply only the Robot part on top of staging's text. Add Robot Framework files (`.robot` and `.resource`) to the sentence now at line 143, leave the Language and notebook support | See Language coverage above row alone, and delete the Broad language + notebook support row you re-added at line 277. Staging replaced that row with the anchor on purpose.

Second, the parser edge cases, which were the other half of the last round. Two probes against your merged branch.

Robot keyword matching is case-insensitive and treats spaces and underscores as equivalent, but the CALLS target is stored verbatim:

*** Test Cases ***
T1
    prepare_environment
    Prepare Environment

*** Keywords ***
Prepare environment
    Log    x

CALLS targets come out as ['prepare_environment', 'Prepare Environment', 'Log'] against a keyword node named Prepare environment. Two of the three spellings of the same keyword never resolve, so callers, callees and test coverage are wrong for them. Normalise the key on both the keyword node and the edge target, or add a Robot-specific resolver.

Pipe-delimited files, which are valid Robot syntax, produce junk nodes:

*** Test Cases ***
| T2 |
|    | Log | hello |

yields a Test named | T2 | and a second Test named |. Either strip the pipe framing or skip pipe-delimited files rather than indexing them wrongly.

The one test you have covers the happy path only. Add a case for each of the two above.

Removing needs-rebase since it merges cleanly, adding changes-requested.

@tirth8205 tirth8205 added changes-requested Approach needs a change before merge and removed needs-rebase Branch no longer merges into staging labels Sep 16, 2026
@Bakul2006

Copy link
Copy Markdown
Contributor Author

Dependency setup is now resolved using uv sync --extra dev. The full test suite completed successfully through collection and execution.

Results: 3,957 passed, 732 skipped, 46 deselected, 2 xfailed, 2 xpassed, 2 failed.

The remaining failures are:

test_hybrid_search_ranks_impact_radius_nodes_first
test_windows_server_still_prewarms_before_mcp_run

These are genuine behavioral/environment-specific failures rather than dependency/collection errors. No source code or tests were modified.
should i commit these or solve up for 2 failling tests

@tirth8205

Copy link
Copy Markdown
Owner

The type-check job is red and this branch turned it red. code_review_graph/main.py lines 71 and 73 fail mypy:

code_review_graph/main.py:71: error: Module has no attribute "WindowsSelectorEventLoopPolicy"  [attr-defined]
code_review_graph/main.py:73: error: Module has no attribute "WindowsSelectorEventLoopPolicy"  [attr-defined]

Staging reports Success: no issues found in 77 source files. Past the type error, that block assigns onto the stdlib asyncio module at import time. On macOS and Linux, hasattr(asyncio, "WindowsSelectorEventLoopPolicy") is False before importing code_review_graph.main and True afterwards, holding _UnixDefaultEventLoopPolicy. Any library in the same process that uses that attribute as a Windows check will take the Windows branch.

The search.py edit regresses ranking. identifier_words(symbol or row["name"]) becomes identifier_words(row["name"]), which drops the class half of a qualified symbol from coverage scoring. On a repo holding UserService.authenticate, authenticate_request and service_authenticate_helper, the query user service authenticate ranks UserService.authenticate first on staging at 0.0317 and third here at 0.0212, behind the other two. UserService authenticate falls from 0.1311 to 0.0410. That partially reverts the search-quality work in #1009.

Both edits chase failures that are not real. On unmodified origin/staging in a clean environment:

uv run pytest -k "hybrid_search_ranks_impact_radius_nodes_first or windows_server_still_prewarms_before_mcp_run" -q
# 2 passed

They pass on your branch too. So neither option. Revert both files to staging.

The two parser probes from last round reproduce verbatim against a real code-review-graph build, not just against the parser in isolation. The prepare_environment / Prepare Environment file still produces three CALLS rows with target_resolution=unresolved sitting next to the node tests_rf/login.robot::Prepare environment. The pipe-delimited file still writes Test nodes named | T2 | and | into graph.db. tests/test_parser.py is byte-identical to the version I reviewed last time, so the test per probe is missing as well.

README.md is unchanged. grep -oE '^\| \*\*[^*]+\*\*' README.md | sort | uniq -d still returns the same 20 rows, and the table is 56 rows against staging's 32. The only Robot mention left is line 282, inside the Broad language + notebook support row I asked you to delete, so deleting that row leaves Robot Framework documented nowhere. The last commit also pasted the same 24 English rows into README.hi-IN.md, README.ja-JP.md, README.ko-KR.md and README.zh-CN.md, taking each from 32 to 56 rows, so four translated files now carry an untranslated copy of their own feature table.

The feature itself works. .robot and .resource both map to robot, staging parses neither, an exactly-spelled keyword call resolves end to end with target_resolution=direct, and 5,000 keywords parse in 0.04s. The branch merges into staging with no conflicts.

What would make it mergeable:

  • Revert code_review_graph/main.py and code_review_graph/search.py to staging.
  • Normalise the Robot keyword key on both the node name and the CALLS target, case-insensitive and treating spaces and underscores as equal.
  • Strip the pipe framing or skip pipe-delimited files.
  • One test for each of those two.
  • Fix README.md as described last round, and revert the four translated READMEs.

Two more parser bugs. A test case and a keyword sharing a name in one file collide on <file>::<name> and the Test node is silently overwritten, so the test case disappears from the graph. FOR, IF, ELSE and END become CALLS edges, now mirrored as TESTED_BY, so the graph claims END is tested by the test.

@tirth8205

Copy link
Copy Markdown
Owner

Thank you for this PR.

I merged it with the latest staging. Then I ran mypy, the tests and a small Robot build.

Before I can merge it, please fix these:

  1. mypy fails at code_review_graph/main.py:73 and :75. This block also changes asyncio on macOS. Please remove it. The two tests it targets already pass on staging.
  2. code_review_graph/search.py:995-1008 makes ranking worse. For user service authenticate, UserService.authenticate drops from first to second. Please revert this file to staging.
  3. Robot ignores case, spaces and underscores in keyword names. The parser does not (parser.py:4572). So prepare_environment and PREPARE ENVIRONMENT never link to Prepare environment.
  4. IF, FOR and END are saved as calls. In ${result}= Prepare environment, the call is saved as ${result}=. The real keyword is lost.
  5. Pipe rows like | T2 | create Test nodes named | T2 | and | (parser.py:4521).
  6. A test case and a keyword both named Shared in one file clash. Only the keyword stays.
  7. Resource keywords.resource is saved as a plain name (parser.py:4536). So importers_of on that file finds nothing.
  8. Please add one test for each of items 3 to 7.
  9. README.md has 20 duplicate feature rows. Please remove them. Keep the Robot Framework mention in the language list. The four translated READMEs got 24 English rows. Please revert them to staging.

After you push the fix, I will test it again.

@Bakul2006

Copy link
Copy Markdown
Contributor Author

I completed the investigation of all nine broader-suite failures.

  • Search ranking: The failure reproduces on staging when the checkout path has the same shape as the PR workspace. Removing the ten PR-added indexed entities does not change the ranking. search.py and the failing test are identical to staging, so I found no evidence that the parser changes caused this failure.
  • README parity: All four checks fail because the main README contains two additional unique rows, Graph diff and Memory loop. The translated READMEs are restored exactly to staging, as requested. Should these two features also be added to the translations, or should the parity mismatch be handled separately?
  • Windows event-policy test: The same failure reproduces on staging under Linux/Python 3.14.2 before main() runs.
  • Subprocess handshakes: All three failures reproduce on staging. They pass when the declared anyio dependency directory is exposed through PYTHONPATH; the issue is the isolated subprocess environment, not server behavior.

The five new regression tests, parser suite, main/server tests, mypy, Ruff, and git diff --check pass. I have not changed the search implementation or weakened the failing tests to force a green suite.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes-requested Approach needs a change before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Can we add further support for the Robot Framework language?

3 participants