Repository navigation
Pingora edge policy has no way to admit declared research/data artefacts, forcing research binaries under docs/ or leaving PRs unmergeable #2193
Description
Gap
scripts/ci/pingora_edge_policy.py admits binary or non-UTF-8 content only when the path carries a doc/docs/documentation component (DOCUMENTATION_DIRECTORIES, line 52; _is_known_documentation_path, line 168), plus the evidence/figures publication directories added by #2149. Everything else is a content-policy violation.
In a research repository the binary artefacts that most need preserving are not documentation: raw response workbooks, SPSS .sav files, serialized model objects, compressed numeric arrays. ContextualWisdomLab/late-life-anxiety-reanalysis keeps these under local/ and an evidence-preservation path by deliberate, owner-approved design. The scanner rejects them on path shape alone and cannot distinguish a preserved research artefact from an unreviewed binary blob.
A content-policy failure is a failure about the change itself, so it is not eligible for the infrastructure bypass. The only routes left are to relocate research data under a docs/ path, which misrepresents what it is, or to leave the pull request permanently unmergeable. That repository already took the first route once, relocating 66 images under docs/ purely to satisfy the path rule.
Ask
Let a repository declare paths that hold research or data artefacts, and admit binary content there on the same terms as documentation. This is not a general weakening: unreviewed binaries in application code must still fail.
Design constraint that must not be missed
evaluate_pull_request fetches file content only at head_sha (_load_file_content, line 416; the function takes no base ref today). A declaration read from the pull-request head would be self-authorizing: one pull request could add both the declaration and the binary, and the check would admit itself.
The declaration must therefore be resolved from the base branch at merge base, so admission depends only on already-reviewed state, and evaluate_pull_request needs the base ref threaded in for that purpose. A pull request that introduces or widens the declaration gets no benefit from it until that change itself is reviewed and merged.
Suggested shape
- A declaration file in the consumer repository, read from the base ref: an explicit list of path prefixes, no globs that can widen unexpectedly, with a documented cap on entry count and prefix depth.
- Admission under a declared prefix still requires the existing magic-byte and completeness evidence; the declaration only replaces the path-shape test, never the content test.
- Runtime forms (
_runtime_path_rule) stay rejected inside declared prefixes, exactly as they are insidedocs/today. - Violation text names the declared prefix that admitted a file, so a reviewer can see which reviewed declaration was relied on.
Reporter constraints
The repository is private, and the study owner reviewed the presence of participant-level files and explicitly declined cleanup, so this is not a question about whether the data belongs there. The affected paths are stable, so a declared list would not need frequent changes.
Status of the immediate case
Not blocked. The pull request that surfaced this was closed as superseded (all three files in its diff were already byte-identical on main), so no bypass is needed and none should be performed: the failure was a genuine content-policy failure, not an infrastructure one. This issue is to fix the rule before the next preservation change hits it.
Reported by the session coordinating ContextualWisdomLab/late-life-anxiety-reanalysis. Related: #2116, #2149.
🤖 Generated with Claude Code
Activity
seonghobae commentedon Sep 14, 2026
Design input from measurements I took on
origin/mainwhile helping the same consumer repository (detail and method in #2116, 2026-09-14): the current rule has an unstable case for large text artefacts, not only for binary ones. A declaration keyed on "binary content" would not close it.Measured with
_is_binary_documentation_assetand_needs_content_scancalled directly, all with adocs/path component present:_is_binary_documentation_asset_is_documentation_or_source_fixturedocs/figures/fig1.pngdocs/figures/fig1.pdfdocs/figures/fig1.svgBINARY_DOCUMENT_MAGICis{.hwpx, .pdf, .png}(:44-48) andDOCUMENT_SUFFIXESis{.md, .mdx, .rst, .adoc, .txt}(:34);.svgis in neither, so it receives no exemption at any path. It passes today only incidentally:_needs_content_scan(:645-661) returns False whilepatch_availableis true, so the file is never loaded. GitHub omitspatchfor very large text files (this module's own comment at:437-443), and past that threshold the same SVG is loaded and scanned; over the Contents API ceiling it raisesContentSizeExceededError, which is caught only in_binary_documentation_evidence_confirms(:454) and forgiven only for.pdf(:455). A non-asset SVG never reaches that handler.Consequence for the design in this issue: admission currently flips on file size, a property outside the policy's intent, for any text-serialized artefact. Research repositories hold several of these — SVG figures, large CSV/TSV, JSON-serialized model output, XML instrument exports — all valid UTF-8 and therefore never "binary", yet all capable of crossing the diff threshold.
Suggested adjustment to the ask: let the declared paths admit by declaration, independent of suffix and of whether the content decodes as UTF-8, rather than extending
BINARY_DOCUMENT_MAGIC. That also keeps the acceptance criteria checkable — a declared path admits a 5 MB.svgand a 5 MB.savon identical terms, and an undeclared path admits neither.The base-at-merge-base resolution constraint in the issue body is the right one and is unaffected by this; it just needs to gate a path rule rather than a content-type rule.
I am not the writer here and am not taking this lane:
scripts/ci/pingora_edge_policy.pycurrently has 7 open PRs against it (#1398, #1450, #1678, #1879, #1946, #2047, #2149). Posting as evidence so the design accounts for the text case.feat(pingora): admit declared research/data artifact paths (#2193) (#…
seonghobae commentedon Sep 14, 2026
Verified #2194 on
main@91be64429against my comment above. My suggestion was considered and correctly superseded — closing that thread so it is not re-opened later as unaddressed.I proposed admitting declared paths independent of suffix and UTF-8 decodability. The merged rule does something better reasoned: a declared prefix replaces only the path-shape test, and for a suffix with no
BINARY_DOCUMENT_MAGICentry the admission still requires the strict complement of the decode this module already performs. Checked the implementation enforces the stated guarantee rather than only documenting it —_binary_documentation_evidence_confirmsdecodes the fetched bytes and admits only onUnicodeDecodeError, so valid UTF-8 is never treated as a binary artefact.Behaviour confirmed by calling the merged code directly (
declared_prefixes=("local/data/",),patch_available=False):local/data/responses.savlocal/data/figure.svglocal/data/table.csvdocs/figures/fig1.svgSo the text-artefact case I raised resolves as deliberately out of scope, not as an oversight: a UTF-8-decodable artefact under a declared prefix still reaches the content scan, because inspecting exactly that content is what this module is for. That is the right trade — it keeps the central guarantee intact and avoids an open-ended magic-byte catalogue.
The residual I measured in #2116 is unchanged and is not a defect in this fix: for text-serialized artefacts, whether the scan happens at all still depends on whether GitHub returned a diff patch, which is a function of file size. It is a property of
_needs_content_scan, predates this issue, and would need its own justification to change. Recording it here only so the boundary is explicit; I am not opening anything for it.