Repository navigation
Add sourced mechanical properties to the filament schema - #893
Conversation
An optional `mechanical` array on filament.json (schemas/mechanical_schema.json): one entry per published value (tensile, flexural, impact, heat deflection) with its print direction, unit as published, standard, condition, test-bar details and the sustainability-style source fields. Follows OpenFilamentCollective#768: data-quality checks (sources, value in quote, unit fits the property), CSV/SQLite columns, key order and tracker stripping in style_data, WebUI display and editor, manual section and tests. The number-in-quote check also accepts thousands separators (7,250 psi). Refs OpenFilamentCollective#892 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
coffandro
left a comment
There was a problem hiding this comment.
Thanks for this. The mechanical schema and the editor look solid. Below are the issues I found. Inline comments cover the changed lines. Two concerns are about code this PR doesn't touch, so they're here:
AI review guard skips mechanical. In ofd/review/suggest.py (~L138), the "never suggested" filter blocks pointers containing sustainability or container_details, but not mechanical. A review model could emit an enrichment like /mechanical/0/value with a guessed number, and it would become a PR suggestion. That breaks the "copy, never compute" rule this schema enforces. ofd/review/triage.py (~L73) and ofd/review/sustainability.py (~L448) have the same sustainability-only checks.
No source gate or auto-merge exclusion for mechanical-only PRs. The "Sustainability sources" gate (ofd/review/sustainability.py ~L60) and auto-merge eligibility (eligibility.py ~L285) only look at sustainability paths. A PR that only adds mechanical entries with a wrong source_url, where the quote check still passes, gets no human source review and can be auto-merged. The PR description mentions this gap. I'd close it here or in a follow-up before contributors start using the field.
I think the two above and the two number-matching comments on _number_pattern should be fixed before merge. The rest are smaller.
🤖 Generated with Claude Code
- Number in quote: a value no longer matches part of a longer number (5 in 5.85, 7 in 7,250). Grouped thousands only in MPa, psi, ksi and J/m; elsewhere a comma is a decimal comma. Sustainability quotes never group. Same rules in the WebUI. - AI review: mechanical values are never suggested, mechanical findings are never dropped, and the "Sustainability sources" gate checks mechanical entries against their source, so E15 blocks auto-merge. - style_data also cleans the specimen object. - A repeated entry now also compares the specimen. - WebUI: inline "... is required" hint, untouched new entries are dropped on save, one editable-copy helper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Automated review of
Maintainers: ✅ LGTM & merge · workflow run |
|
Thanks for the review. All points are fixed in 1511610.
The bot's "Sustainability blocked" on this PR is expected: the gate fails closed when Tests: 🤖 Addressed by Claude Code |
edc8d70
into
OpenFilamentCollective:main
|
LGTM! Thanks for the contribution, it'll be up on SP and the dataset at about 00:00 Danish time. |
Schema part of #892. The data follows in a separate PR on top of this one.
What
An optional
mechanicalarray onfilament.json(schemas/mechanical_schema.json). One entry per published value: property, print direction, value, ± spread, unit, standard, dry or conditioned, test-bar details (specimen), and the same source fields as sustainability (source_type,source_url,source_quote,retrieved,notes).It follows #768:
$ref.ofd-validatorresolves it as is (tested).data_quality.py): reuses the sustainability source checks. Errors: missing source fields, value or ± not in the quote,retrievedin the future, a unit that doesn't fit the property. Warnings: a print direction on moulded bars, a repeated entry (same property, direction, condition, standard, unit and specimen).Sustainability sourcesgate also checks mechanical entries against their source (the quote on the page, the value in the quote), and a PR that touches them never auto-merges. The AI review never suggests mechanical values, and its mechanical findings are never dropped. The gate fails closed on this PR because it changes rule files (data_quality.py,sustainability.py), so a maintainer must review it.MPa,psi,ksiandJ/mit also matches grouped thousands (7250↔7,250), as sheets in psi write them. In other units a comma stays a decimal comma. A value no longer matches part of a longer number (5in5.85,7in7,250). Sustainability quotes never group.mechanicalJSON column in SQLite and CSV. The API carries the field as is.source_url;specimencleaned like the other fields.SourcedEntryFields), the schema endpoint, the same checks client-side, and tests. A new entry left empty is dropped on save.Notes
directiongainsXZ(some sheets test on-edge bars) and usesunknowninstead of"".checkSourcedEntrychecked every value field exceptvalue_kg_co2eas a percentage. It now checks onlypct,recycled_pctandreduction_pct, like the Python side.method: "epd"). It now starts on "Choose…" and shows an inline "… is required" error until a value is picked.Testing
pytest: 547 passed. The 2 failures intest_search_index_exporter.pyalso fail onmain.ruff check,ruff format --check: clean.ofd validate: passes with no errors.style_data --dry-run: 0 files changed.vitest1071 passed,npm run check0 errors,npm run buildOK.🤖 Generated with Claude Code