Skip to content

Conversation

@ppinchuk
Copy link
Collaborator

@ppinchuk ppinchuk commented Nov 8, 2025

Pull Request Overview

This PR adds comprehensive test coverage for COMPASS parsing utilities and updates the CI workflow to trigger on test file changes.

  • Adds extensive test coverage for parsing utility functions including LLM response cleaning, ordinance counting, and config loading
  • Fixes test function naming (renamed from test_sync_retry to test_llm_response_as_json)
  • Updates CI workflow to trigger on changes to test files

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/python/unit/utilities/test_utilities_parsing.py Adds comprehensive test coverage for parsing utilities including clean_backticks_from_llm_response, extract_ord_year_from_doc_attrs, num_ordinances_in_doc, num_ordinances_dataframe, ordinances_bool_index, and load_config functions. Also corrects the test function name from test_sync_retry to test_llm_response_as_json.
.github/workflows/ci-python.yml Updates CI workflow triggers to include tests/** path, ensuring CI runs when test files are modified

@ppinchuk ppinchuk requested a review from castelao as a code owner November 8, 2025 22:24
@ppinchuk ppinchuk self-assigned this Nov 8, 2025
@ppinchuk ppinchuk added enhancement Update to logic or general code improvements topic-python-general Issues/pull requests related to python p-high Priority: high labels Nov 8, 2025
@ppinchuk ppinchuk added this to the Finishing touches for OSS milestone Nov 8, 2025
@ppinchuk ppinchuk requested a review from Copilot November 8, 2025 22:32
Copy link
Contributor

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.

Moved to top-level description


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

castelao
castelao previously approved these changes Nov 15, 2025
Copy link
Member

@castelao castelao left a comment

Choose a reason for hiding this comment

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

Thanks for adding this tests. I didn't check the logic of most of the values themselves, but everything make sense.

It might be worth merging #342 first, so this PR here can validate the setup used there.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.50%. Comparing base (a55dba5) to head (41af9db).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #330      +/-   ##
==========================================
+ Coverage   36.08%   36.50%   +0.42%     
==========================================
  Files          45       45              
  Lines        4240     4240              
  Branches      380      380              
==========================================
+ Hits         1530     1548      +18     
+ Misses       2687     2670      -17     
+ Partials       23       22       -1     
Flag Coverage Δ
unittests 36.50% <ø> (+0.42%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ppinchuk ppinchuk merged commit 73d6592 into main Nov 15, 2025
14 checks passed
@ppinchuk ppinchuk deleted the pp/parsing_unit_tests branch November 15, 2025 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Update to logic or general code improvements p-high Priority: high topic-python-general Issues/pull requests related to python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants