Skip to content

Conversation

@svillegas-cdd
Copy link
Contributor

  • Group rows by the combination of folio + tipo docto + rut to handle "Otros Impuestos".
  • Solves the issue where some rows with the same folio but different rut or tipo docto were missing.

Ref: https://app.shortcut.com/cordada/story/17311/

@svillegas-cdd svillegas-cdd self-assigned this Nov 13, 2025
@svillegas-cdd svillegas-cdd added bug Something isn't working component: rcv labels Nov 13, 2025
Copilot finished reviewing on behalf of svillegas-cdd November 13, 2025 15:47
Copy link

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

This PR refactors the RCV (Registro de Compra y Venta) CSV parsing logic to correctly group rows by the combination of folio + tipo docto + rut instead of just folio. This change ensures that rows with the same folio but different document types or RUT values are treated as separate entries, which is necessary for properly handling "Otros Impuestos" (Other Taxes) data.

  • Changed the grouping key from folio to entry_key (combination of folio, tipo docto, and rut)
  • Added support for different RUT field names between RCV Venta and RCV Compra schemas
  • Updated test expectations to verify the new grouping behavior with edge cases

Reviewed Changes

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

File Description
src/cl_sii/rcv/parse_csv.py Refactored grouping logic to use composite key (folio + tipo docto + rut) instead of just folio, and added rut_key variable for schema-specific field names
src/tests/test_rcv_parse_csv.py Expanded test to verify 5 distinct entries are correctly parsed with the new grouping logic, including cases with same folio but different tipo docto or rut
src/tests/test_data/sii-rcv/RCV-venta-extra-empty-impuestos-rows.csv Added 3 new test rows to cover edge cases: same folio with different tipo docto (row 6), same folio with different rut (row 7), and additional otros impuestos row (row 8)

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

@svillegas-cdd svillegas-cdd force-pushed the task/sc-17311--lib-cl-sii-python-error-en-parseo-de-rcv branch from 4e36668 to 0c4ddbd Compare November 13, 2025 15:59
@svillegas-cdd svillegas-cdd marked this pull request as ready for review November 13, 2025 15:59
@svillegas-cdd svillegas-cdd requested a review from a team as a code owner November 13, 2025 15:59
Copilot finished reviewing on behalf of svillegas-cdd November 13, 2025 16:07
Copy link

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 3 out of 3 changed files in this pull request and generated 1 comment.


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

- Updated to handle case when a main row is encountered and the entry_key already exists and the code doesn't update the stored row.
- Changed the grouping key from folio to entry_key (combination of folio, tipo docto, and rut)
- Solves the issue where some rows with the same `folio` but different `rut` or `tipo docto` were missing.

Ref: https://app.shortcut.com/cordada/story/17311/
@svillegas-cdd svillegas-cdd force-pushed the task/sc-17311--lib-cl-sii-python-error-en-parseo-de-rcv branch from 0c4ddbd to 9898051 Compare November 13, 2025 18:35
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
69.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.79%. Comparing base (7e0486f) to head (9898051).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
src/cl_sii/rcv/parse_csv.py 71.42% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #937      +/-   ##
===========================================
- Coverage    88.90%   88.79%   -0.11%     
===========================================
  Files           40       40              
  Lines         3759     3767       +8     
  Branches       389      390       +1     
===========================================
+ Hits          3342     3345       +3     
- Misses         265      267       +2     
- Partials       152      155       +3     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@svillegas-cdd svillegas-cdd merged commit 34f50d9 into develop Nov 13, 2025
25 of 27 checks passed
@svillegas-cdd svillegas-cdd deleted the task/sc-17311--lib-cl-sii-python-error-en-parseo-de-rcv branch November 13, 2025 18:43
@svillegas-cdd svillegas-cdd mentioned this pull request Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working component: rcv

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants