Fm/protected donor dcicutils

[aschroed]

Purpose

Add the dcicutils-side ProtectedDonor workbook analysis and selective transformation used by submitr.

Feature summary

• Supports the current protected-item sheets: Demographic, DeathCircumstances, FamilyHistory, MedicalHistory, and TissueCollection.
• Classifies plain Donor and ProtectedDonor references across actual workbook data rows.
• Allows mixed references and transforms only referenced Donors that still need conversion.
• Validates missing workbook Donors clearly.
• Accepts existing ProtectedDonors found in the workbook or through the portal.
• Leaves unreferenced Donor rows untouched and supports incremental transformation alongside existing ProtectedDonor sheets.
• Integrates with CustomExcel while keeping generic structured-data parsing separate.

Remediation updates

• Hardened transformed-workbook target protection so an earlier save cannot silently authorize a later overwrite of the same input/output pair.
• Preserved existing CustomExcel mapping behavior; mapped fields are materialized by the submitr data path rather than by the raw ProtectedDonor workbook transform.
• Added regression coverage for existing-target protection, repeated saves, and the relevant CustomExcel integration.
• Corrected the readthedocs declaration for dcicutils.submitr.donor_transformer.

Review focus

• Reference classification and incremental transformation behavior.
• Workbook and portal existence checks and error handling.
• Generated ProtectedDonor row contents and portal-unique fields.
• Existing-target safety and CustomExcel integration.
• Focused tests covering mixed, missing, portal-backed, already-transformed, and repeated-save cases.

The downstream submitr implementation and payload remediation are tracked in smaht-dac/submitr#48.

[coveralls]

Coverage Report for CI Build 37041710615

Coverage increased (+0.9%) to 75.501%

Details

• Coverage increased (+0.9%) from the base build.
• Patch coverage: 31 uncovered changes across 2 files (388 of 419 lines covered, 92.6%).
• 3 coverage regressions across 3 files.

Uncovered Changes

File	Changed	Covered	%
dcicutils/submitr/donor_transformer.py	365	341	93.42%
dcicutils/submitr/custom_excel.py	44	37	84.09%
Total (3 files)	419	388	92.6%

Coverage Regressions

3 previously-covered lines in 3 files lost coverage.

File	Lines Losing Coverage	Coverage
dcicutils/command_utils.py	1	58.57%
dcicutils/progress_bar.py	1	74.61%
dcicutils/structured_data.py	1	71.96%

---

Coverage Stats

Relevant Lines:	16025
Covered Lines:	12099
Line Coverage:	75.5%
Coverage Strength:	0.76 hits per line

---

💛 - Coveralls

[aschroed]

Implemented and pushed generic JSON schema ordering fix in commit a61b3ac on fm/protected-donor-dcicutils.

Summary:

• Shared canonical Schema.type_name ordering between workbook sheets and multi-schema JSON top-level keys.
• Supplied order now reorders recognized JSON keys while preserving original key spelling and stable unknown-key order.
• No-order behavior, single-schema JSON behavior, and nested record field ordering remain unchanged; no ProtectedDonor-specific logic was added.

Tests:

• Focused ordering tests: 5 passed.
• StructuredDataSet + ProtectedDonor suites: 111 passed.
• flake8 on dcicutils/structured_data.py and test/test_structured_data.py: passed.

Adversarial review outcome:

• Challenged normalization, canonical-key collisions, unknown-key stability, no-order and single-schema paths, nested fields, workbook compatibility, and regression coverage.
• Added coverage for canonical spelling collisions and nested field order; all findings were addressed.

[willronchetti]

Request changes on reviewed head 1abf5d658ad1c218fbe482d6434d29707c1b3d3b. The 132 focused/related tests pass, but the following uncovered cases were reproduced with synthetic workbooks and mocked portal access.

1. Preserve copied fields - dcicutils/submitr/donor_transformer.py:292

   Writing only destination headers silently drops incoming fields absent from an existing ProtectedDonor sheet. Please extend the headers or reject the mismatch before rewriting links.

2. Keep generated rows visible - dcicutils/submitr/donor_transformer.py:290

   max_row includes formatted empty cells, but the reader stops at the first empty row. Newly appended records can therefore disappear from parsed data while references point to them. Please append before the logical terminator or reject this shape.

3. Accept supported identifiers - dcicutils/submitr/donor_transformer.py:150-152

   UUID/accession references to existing ProtectedDonors resolve successfully in the base reader but are rejected here. Please resolve identity and type rather than classify references solely by submitted-ID spelling.

4. Match reader termination - dcicutils/submitr/donor_transformer.py:306

   This scans references beyond the empty-row terminator, so stale content can reject an otherwise valid workbook. Please use the same logical-row boundaries as ingestion.

5. Distinguish lookup failure from absence - dcicutils/submitr/donor_transformer.py:410-413

   A permission error or outage is reported as a missing ProtectedDonor. Please preserve lookup failures and reserve 'absent' for definitive not-found results.

6. Avoid counting-pass saves - dcicutils/submitr/custom_excel.py:119-122

   Progress-enabled loading saves during counting, then fails during parsing because its output already exists. Please suppress counting-pass writes or reuse the instance while preserving no-clobber protection. The current submitr staging caller avoids this, but default library callers do not.

[aschroed]

Thanks for the review. All six concerns are addressed (fixes are on top of this PR's head; pushed via the no-mistakes pipeline):

1. Preserve copied fields - an existing ProtectedDonor sheet now gets headers added for any copied fields it lacks. If content lies beyond its header columns, the transform raises before changing anything.
2. Keep generated rows visible - rows are appended directly after the last row the reader ingests (its first-empty-row boundary), not at max_row. A sheet with content after that first empty row is rejected before mutation.
3. Supported identifiers - references without the ProtectedDonor token (UUID/accession) are resolved by identity and type: workbook ProtectedDonor submitted_id/uuid/accession first, then a typed /ProtectedDonor/<id> portal lookup (with @type checked when returned). A different type is not accepted; no new identifier kinds beyond what the base reader accepts.
4. Match reader termination - all scans, rewrites, and appends (Donor and reference sheets) use one shared logical-row helper mirroring the reader's empty-row rule, so stale content past the terminator neither rejects a valid workbook nor gets rewritten.
5. Lookup failures vs absence - lookups return a typed outcome (found / absent / permission denied / failed). Only a definitive not-found is "absent"; permission and outage/other errors raise ProtectedDonorLookupError and are reported on the analysis result.
6. Counting-pass saves - StructuredDataSet._load_excel_file now opens the workbook once and reuses it for the progress counting and parsing passes, so only one save occurs; no-clobber protection is unchanged (a second construction with an existing output path still fails).

Validation: 20 new regression tests in test/test_protected_donor_transform.py (they fail against the previous code), 142 related tests pass (protected donor, structured data, custom excel, progress bar), and flake8 is clean.

[aschroed]

The validated follow-up changes from PR 341 are now on this PR's branch at the same tested head.

They address all six review concerns: preserving copied fields and rejecting unsafe header layouts, matching logical row termination for generated rows and reference scans, resolving supported UUID/accession identifiers by identity and type, distinguishing lookup failures from definitive absence, and preventing counting-pass saves from breaking parsing. The final version is 8.19.1 with the corresponding changelog entry. The validation suite passed, including 42 focused tests, the related test coverage, and flake8.

PR 341 is being closed as the duplicate now that its validated changes are here.