Skip to content

Fix unit tests broken by the transition to TRAPI 1.6.0 in kp_info_cacher (which means Expand uses Retriever)#2713

Merged
saramsey merged 20 commits intomasterfrom
issue-2712
Apr 4, 2026
Merged

Fix unit tests broken by the transition to TRAPI 1.6.0 in kp_info_cacher (which means Expand uses Retriever)#2713
saramsey merged 20 commits intomasterfrom
issue-2712

Conversation

@saramsey
Copy link
Copy Markdown
Member

@saramsey saramsey commented Apr 3, 2026

@edeutsch I could use your help reviewing this PR. This is for issue #2712, the broken unit tests in ARAX master branch since the transition to ARAX only talking to TRAPI 1.6.0 KPs.

@saramsey saramsey requested a review from edeutsch April 3, 2026 21:32
Copy link
Copy Markdown
Collaborator

@edeutsch edeutsch left a comment

Choose a reason for hiding this comment

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

This looks excellent to me, except that one test is still failing for me:

$ pytest -v test_ARAX_workflows.py::test_FET_example_3
...
>       assert response.status == 'OK'
E       AssertionError: assert 'ERROR' == 'OK'
E         
E         - OK
E         + ERROR

test_ARAX_workflows.py:254: AssertionError

@saramsey
Copy link
Copy Markdown
Member Author

saramsey commented Apr 3, 2026

I will work on that. Also I will add the old tests in an archive folder, this same PR.

@saramsey
Copy link
Copy Markdown
Member Author

saramsey commented Apr 3, 2026

Hmm, test test_ARAX_workflows.py::test_FET_example_3 seems to work for me:
Screenshot 2026-04-03 at 3 12 26 PM

@edeutsch can you please run this command, wherever you were running your tests? (assuming your shell is bash):

pytest -vv -s --cache-clear test/test_ARAX_workflows.py::test_FET_example_3 2>output.log 1>&2

and post output.log as an upload to this PR?

@bazarkua bazarkua requested a review from Copilot April 3, 2026 23:27
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates ARAX’s test harness and query expansion utilities to work with TRAPI 1.6.0-era KPs (notably infores:retriever), aiming to restore passing unit tests on master after the KP transition.

Changes:

  • Adds pytest.ini configuration to exclude legacy/slow/unrelated test directories from default pytest discovery.
  • Updates many unit tests and workflow examples to use infores:retriever and updated curies/predicates/categories aligned with TRAPI 1.6.0 expectations.
  • Refactors portions of Expand/Filter_KG logic for TRAPI 1.6.0 structures (qnode_keys/qedge_keys handling, subclass edge handling, KP selection).

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
pytest.ini Excludes legacy/archived and non-unit-test dirs from pytest discovery.
notes/arax-maintenance-sop.md Updates unit-test run instructions and expected test counts.
code/code-archive/old-arax-tests/test_ARAX_translate.py Adds archived legacy translate tests (now excluded by pytest.ini).
code/code-archive/old-arax-tests/test_ARAX_synonymizer.py Adds archived legacy synonymizer tests (now excluded by pytest.ini).
code/code-archive/old-arax-tests/test_ARAX_standup_queries.py Adds archived legacy standup workflow tests (now excluded).
code/code-archive/old-arax-tests/test_ARAX_ranker.py Adds archived legacy ranker tests (now excluded).
code/code-archive/old-arax-tests/test_ARAX_query.py Adds archived legacy query tests (now excluded).
code/code-archive/old-arax-tests/test_ARAX_messenger.py Adds archived legacy messenger tests (now excluded).
code/code-archive/old-arax-tests/test_ARAX_json_queries.py Adds archived legacy JSON query tests (now excluded).
code/code-archive/old-arax-tests/test_ARAX_infer.py Adds archived legacy infer tests (now excluded).
code/code-archive/old-arax-tests/test_ARAX_filter_results.py Adds archived legacy filter_results tests (now excluded).
code/code-archive/old-arax-tests/test_ARAX_filter_kg.py Adds archived legacy filter_kg tests (now excluded).
code/code-archive/old-arax-tests/test_ARAX_connect.py Adds archived legacy connect tests (now excluded).
code/code-archive/old-arax-tests/conftest.py Adds archived legacy pytest config/markers (now excluded).
code/ARAX/test/test_ARAX_workflows.py Updates workflow tests to use infores:retriever and revised categories/predicates.
code/ARAX/test/test_ARAX_translate.py Updates translate tests (curies + allowlists) for infores:retriever.
code/ARAX/test/test_ARAX_resultify.py Updates resultify tests to use infores:retriever and revises some test queries.
code/ARAX/test/test_ARAX_overlay.py Updates overlay tests (categories/predicates/KP) for TRAPI 1.6.0 behavior.
code/ARAX/test/test_ARAX_json_queries.py Updates JSON workflow tests to infores:retriever and revised expectations.
code/ARAX/test/test_ARAX_filter_results.py Updates filter_results tests to infores:retriever.
code/ARAX/test/test_ARAX_filter_kg.py Updates filter_kg tests to infores:retriever and revised expected error codes.
code/ARAX/test/test_ARAX_expand.py Updates expand tests to infores:retriever, adjusts curies, removes single-node tests.
code/ARAX/ARAXQuery/Filter_KG/remove_edges.py Improves robustness around missing qnode_keys/qedge_keys, adds logging, refactors locals.
code/ARAX/ARAXQuery/Expand/trapi_querier.py Adjusts QG↔KG mapping behavior, attribute handling, and subclass edge parent-node sourcing.
code/ARAX/ARAXQuery/Expand/kp_selector.py Tweaks KP selection defaults and refines CURIE conversion logic/order.
code/ARAX/ARAXQuery/Expand/kp_info_cacher.py Fixes a debug-log typo.
code/ARAX/ARAXQuery/ARAX_filter_kg.py Makes allowable-parameter computation tolerant of missing qnode_keys/qedge_keys.
code/ARAX/ARAXQuery/ARAX_expander.py Changes post-processing expectations for KP outputs; refactors/limits single-node expansion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@saramsey saramsey changed the title Issue 2712 Fix unit tests broken by the transition to TRAPI 1.6.0 in kp_info_cacher (which means Expand uses Retriever) Apr 3, 2026
@saramsey
Copy link
Copy Markdown
Member Author

saramsey commented Apr 4, 2026

@bazarkua can you get Copilot to re-review this PR, after commit 02df4b6?

@bazarkua bazarkua requested a review from Copilot April 4, 2026 00:25
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

pytest.ini:1

  • norecursedirs is doing a lot of heavy lifting to restrict collection, including excluding at least one source directory (code/ARAX/ARAXQuery/Overlay). A more maintainable approach is typically to set testpaths = code/ARAX/test (and optionally python_files = test_*.py) so pytest only collects intended unit tests, without needing to keep an expanding exclude list. This reduces the chance of accidentally excluding legitimate tests or masking new test directories.
    notes/arax-maintenance-sop.md:1
  • The SOP hardcodes an exact expected unit test count in multiple places (now two different numbers appear in the same doc). This tends to go stale and causes confusion when tests are added/removed. Consider rephrasing to avoid exact counts (e.g., “All standard unit tests should pass”) or centralizing the number in one place.
    notes/arax-maintenance-sop.md:1
  • The SOP hardcodes an exact expected unit test count in multiple places (now two different numbers appear in the same doc). This tends to go stale and causes confusion when tests are added/removed. Consider rephrasing to avoid exact counts (e.g., “All standard unit tests should pass”) or centralizing the number in one place.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@saramsey
Copy link
Copy Markdown
Member Author

saramsey commented Apr 4, 2026

OK, all the pytests are passing (both on my MBP and on arax.ncats.io/devED), and I believe I have address Copilot's concerns with the last few commits. All three Example queries and the example Pathfinder query are running fine, on my MBP and on arax.ncats.io/devED.

@edeutsch @bazarkua @dkoslicki what do you think? OK to merge to master?

@saramsey
Copy link
Copy Markdown
Member Author

saramsey commented Apr 4, 2026

The red "x" for the Test Build is (I think) because it is running the pytests for the code in the master branch, which (obviously) is not working. That's literally the point of this issue.

@saramsey
Copy link
Copy Markdown
Member Author

saramsey commented Apr 4, 2026

I think I just initiated a Test Build from the issue-2712 branch:
https://github.com/RTXteam/RTX/actions/runs/23969548433

@saramsey
Copy link
Copy Markdown
Member Author

saramsey commented Apr 4, 2026

OK, to get the Test Build to actually check out the issue-2712 branch on cicd.rtx.ai, I had to actually modify the CICD-Dockerfile, which is a bit inelegant, but it will have to do for now. Test Build is currently running.

@saramsey
Copy link
Copy Markdown
Member Author

saramsey commented Apr 4, 2026

OK, as you can see, the Test Build passed. I'm pasting a screencap of it here, because as soon as I revert commit 50b7bc4, the Test Build that is kicked off will not pass anymore (because the CICD-Dockerfile clones the master branch RTX.git code). But trust me, it passed!
Screenshot 2026-04-03 at 9 14 59 PM

@saramsey saramsey merged commit 079db18 into master Apr 4, 2026
2 of 3 checks passed
@saramsey saramsey deleted the issue-2712 branch April 4, 2026 04:19
@edeutsch
Copy link
Copy Markdown
Collaborator

edeutsch commented Apr 4, 2026

thank you, Steve!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants