fix(provenance): correct shape dataset ownership - #4
Conversation
Record StructuPath as the creator and copyright holder of the bundled structural-shapes dataset, document its MIT redistribution authorization, and keep the release gate fail-closed unless permission is authorized.\n\nPrepare version 0.2.3 so the reviewed package description and gallery image can be published to npm and reflected on pi.dev.
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe release updates provenance metadata and validation from blocked AISC-derived data to an authorized, ready StructuPath structural shapes dataset. Documentation, package versions, diagnostics, takeoff references, and tests are aligned with the new release state. ChangesProvenance release validation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-data-provenance.py`:
- Around line 25-32: Update the provenance validation around record and datasets
before the shipped_file lookup: require record to be a dict, datasets to be a
one-item list, and its sole item to be a dict, appending structured errors and
using an empty dataset when invalid. Validate that dataset.get("shipped_file")
is a non-empty string before constructing shapes_path, while preserving
fail-closed behavior without dereferencing malformed values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bb342e70-a4e4-4b2a-b535-ceb7b81efd35
📒 Files selected for processing (11)
DATA_PROVENANCE.jsonDATA_PROVENANCE.mdREADME.mdpackage.jsonpyproject.tomlscripts/check-data-provenance.pyscripts/doctor.pyskills/steel-takeoff/SKILL.mdskills/steel-takeoff/scripts/lookup-member.shtests/test_data_provenance.pytests/test_installed_scripts.py
Validate the provenance document shape and shipped-file field before dereferencing either value. Malformed records now produce structured audit errors instead of exceptions, with regression coverage for invalid roots, datasets, and paths.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_data_provenance.py (2)
57-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse direct module assignment instead of
setattr.Ruff B010 flags this constant-attribute
setattrcall; direct assignment is clearer and keeps the lint gate clean.Proposed fix
- setattr(module, "PROVENANCE_RECORD_PATH", record_path) + module.PROVENANCE_RECORD_PATH = record_path🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_data_provenance.py` around lines 57 - 60, Replace the setattr call in the test setup with direct assignment to module.PROVENANCE_RECORD_PATH, preserving the existing temporary record_path value and test flow.Source: Linters/SAST tools
51-55: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover unreadable shipped-file paths explicitly.
These cases cover invalid roots, dataset shapes, and a missing
shipped_file, but not a non-empty path that cannot be read. Add a case such as{"shipped_file": "missing.json"}and assert the structured"declared shape data is missing or unreadable"error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_data_provenance.py` around lines 51 - 55, Extend the validation cases in the test covering DATA_PROVENANCE.json to include a dataset with a non-empty but unreadable shipped_file path, such as “missing.json”. Assert that validation returns the structured error “declared shape data is missing or unreadable”, while preserving the existing invalid-root, dataset-shape, and missing-path cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_data_provenance.py`:
- Around line 57-60: Replace the setattr call in the test setup with direct
assignment to module.PROVENANCE_RECORD_PATH, preserving the existing temporary
record_path value and test flow.
- Around line 51-55: Extend the validation cases in the test covering
DATA_PROVENANCE.json to include a dataset with a non-empty but unreadable
shipped_file path, such as “missing.json”. Assert that validation returns the
structured error “declared shape data is missing or unreadable”, while
preserving the existing invalid-root, dataset-shape, and missing-path cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 303af56d-bbc4-4e47-949c-7b132dfdaab6
📒 Files selected for processing (2)
scripts/check-data-provenance.pytests/test_data_provenance.py
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/check-data-provenance.py
Verify that a non-empty shipped-file path which cannot be read returns the structured fail-closed audit error instead of raising or silently passing.
1 participant
Summary
Verification
python3 -m pytest tests/test_data_provenance.py tests/test_installed_scripts.py tests/test_package_contents.py— 6 passednpm run lintnpm run release:check— 133 passed, 2 deselected; privacy, package, and provenance checks passednpm pack --dry-run --json— version 0.2.3 and required gallery/provenance files presentRelease
After merge, publish
@structupath/pi-steel@0.2.3, create the matching GitHub release, and verify the updated description and gallery image on pi.dev.Summary by CodeRabbit