Skip to content

T099: test_renderer.py, the renderer's 21 composed reds (DJ + VR, HH, CP, PI) (plan 038) - #47

Merged
brettheap merged 10 commits into
mainfrom
r2/t099-renderer
Oct 7, 2026
Merged

brettheap merged 10 commits into
mainfrom
r2/t099-renderer

Conversation

@brettheap

@brettheap brettheap commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Authored by lane openXfactory-3, delegated by lane openxfactory-4 (#656 6021862323)

Arc: neutral-product-standalone-operability

T099: tests/test_renderer.py, the renderer's 21 composed reds (plan 038)

This is slice T099 of T094's map (R2-INV-R9). It works under the holder's rulings at opensoft/openxFactory#656 comment 6021830531 and lane 3's CLAIMED comment 6021862323. Plan 038's tasks.md has no entry for it yet: lane 4 records T094's slices after openxFactory#1251. Until then, the contract is the map's outline row plus those rulings.

  • Realizes (the outline row): DJ + VR 5, HH 12, CP 3 and PI 1, all in tests/test_renderer.py. That is 20 nodes, plus 1 on R9-W3. They are among requirement 9's declared composed tests (T073, F9.1). Each slice clears its own reds so that T073's composed step 5a can read green.
  • Falsifier (the outline row): the composed run of tests/test_renderer.py, alone, reads 0 for the 21 named reds. Its 3 LI nodes arrive with T095 (R9-W2), and its 1 SF node with T104's openDox pin. The unchanged set reads unchanged, and tests/test_declared_exclusion.py reads green lone.
  • Ruled (6021830531; and 6028383410 for the accepted limit below):
  • Decisions:
    • No node of this file asserts a display WORD, so R9-R1 (a) needs no served display here (point 1 below).
    • Every node keeps its name, the inverted pin's (point 4 below) and the collapse node's (Copilot's rename finding, declined below), so the 21 nodes T094's map and T073's declared run account for keep their identity.
  • After: none. Nothing here waits on T095.

What changed: one file, tests/test_renderer.py

  1. DJ + VR, 5 nodes: the model.js harness.
    • Since § 3.4 slice S7 (openDox-code 1e46971), model.js imports ./display.js, which is itself import-free. The harness copied model.js alone, so node read ERR_MODULE_NOT_FOUND. It now copies display.js beside it, as T026, U-7: the allow-list for 12.5's governed suites, with R-1 (a)'s span kind (plan 038, H-2) #43's harness copies the staging model's siblings, and writes a package.json of {"type": "module"}.
    • The six columns are keyed BY ROLE since S7: source, grouping, candidate, selection, submission and completion, where they were docs, clusters, possibles, staged, proposals and realized. The harness and its assertions now read the role keys. Only the keys moved: every count is the one the test always pinned (6/6/5/2/1/1; 16 topic, 1 pick and 1 flow; 8 left after the collapse).
    • No assertion reads a display word, so the harness hands buildFunnelModel no display, and the model takes openDox's neutral one.
  2. HH, 12 nodes: ONE Host line. _raw_get sent two Host lines: putrequest(..., skip_host=False) added one, then putheader("Host", ...) added a second. openDox-code 390e2c2 (T103, openDox-code#80) admits a loopback request only with exactly one, and answered every request with 403 invalid_host ahead of every route. putrequest now skips its own line.
  3. CP, 3 nodes: pinned claims, respelled to the carve's ruled design.
    • The fetch-bearing set (test_data_fetches_target_only_snapshot_and_source_passthrough, and its arithmetic in test_the_session_transport_stays_out_of_the_fetch_bearing_set).
      • Slice S4 (openDox-code 69d2760) split repo-selector.js under RULED Q3 (openxFactory#656 5642758731): "a route constant travels with the binding that calls it". So the set gains projection-index.js (the index read, count 1).
      • repo-selector.js falls from 6 call sites to 2: the refresh POST and the register-projection read, which stay class A.
      • The create-project and edit-project POSTs went with gate-projects.js, which slice S5 leg B (8efb3cf) then moved into this leg's own views. That puts them outside openDox's class-A surface, which is what this pin covers.
      • The apply POST left the bundle: it is openxFactory's own fulfilment lane.
      • The map traced only the new file. The count change was measured here, against openDox-code 84f8ed83.
      • The sibling test's self-quoting guard (its own "finding 24") now quotes six files and six counts, each needle still assembled at runtime.
    • The collapse modifier (test_six_column_default_and_five_column_collapse_marker). openDox-code 3c3a9e3 (openDox-code#35) renamed .collapsed-docs / .only-docs to the role, .collapsed-source / .only-source.
  4. PI, 1 node, W3: test_dynamic_import_is_absent INVERTED.
    • The old pin asserted that no bundle file spelled import( anywhere. It read red on a COMMENT in app.js.
    • The pin now asserts that the bundle's dynamic imports, in CODE, are exactly the carve's ruled loaders of contributed bindings, each argument exactly as written:
      • views/view_extension.js: new URL(binding.module, bundleRoot).href. This is the view registry's loader, written by slice S3 (c5edac8); slice S5 leg B (8efb3cf, RULED Q1 to Q12) mounts the contributed gate loop through it.
      • views/repo-selector.js: "./projection-index.js", from slice S4 (69d2760, RULED Q3).
      • views/intent-binding.js: "./intent-feed.js", from slice S2 (c7ab3d8, RULED Q5).
    • The pin also asserts that the registry's loader resolves against the bundle's own root (new URL("../", import.meta.url)), read from executable code.
    • So none of the three can pull a URL:
      • Two name a bundle-relative sibling literally.
      • The third is bounded where a binding is declared: openDox's viewBinding() refuses an absolute URL, a root-relative path and a .. climb. openDox-code's own tests/test_view_registry.py holds that refusal.
    • How the scan reads code (_without_comments, shaped by Copilot's two findings):
      • It reads each file WHOLE, as a JavaScript lexer reads it. It tracks string, template and regular-expression literal state, and replaces each comment with one space. It KEEPS the literals, so an import( spelled inside one still counts: the scan may over-count, but it never hides a call.
      • So none of these hides a call: a line break or a comment between import and (; a generator method's line that opens with *; a // comment ended by CR, U+2028 or U+2029; a spread (...import(); or U+FEFF before (. Only a member call (a.import() is skipped.
      • It reads .js, .mjs, .cjs, .html, .htm and .css files.
      • Two readings. The default reading keeps every spelling, so it can only over-count: it refuses an unknown import. The executable_only reading skips a spelling inside a literal, and it must equal the same three sites: it proves each ruled loader is an executable call, so a decoy string cannot stand in for a removed loader.
      • The loader's base is read as every executable assignment to bundleRoot (plain or compound, comments not read, literals skipped). It must be exactly bundleRoot = new URL("../", import.meta.url), so neither the old line kept in a comment or a string nor a later reassignment passes.
    • The scan's own regression cases, committed:
      • test_the_dynamic_import_scan_reads_code_as_a_lexer_does: 21 probe modules, each written as exact bytes, with the site map the scan must read;
      • test_the_dynamic_import_scan_reads_every_script_suffix;
      • test_the_executable_reading_counts_only_calls: 6 cases;
      • test_the_loader_base_is_read_from_executable_code: 5 cases.
    • Stated limit: the lexer tells a regular-expression literal from a division by the token before the /, not by a full parse. So a regex literal straight after ) (as in if (x) /re/.test(y)) reads as a division. That is the limit the holder accepted for openDox-code's import-free scan (T025, U-6: the staging model is import-free again (W-1 (A)), and T102's posture-note follow-on (plan 038) openDox-code#91, r4198449977): closing it needs a JavaScript parser.
    • The name is kept on purpose. The node T094's map and T073's declared run account for keeps its identity, and the name still reads true: a dynamic import is absent from every line of code but these three.

The falsifier, composed (R1Q23 (a)), each file alone

The form:

  • openxFactory 92010d3e (the commit tests/composed_host_pin.yaml names), with openDox, openXdox and openXwallet initialized recursively, and its scripts/ on PYTHONPATH;
  • openDox installed from openDox-code main 84f8ed83 (T025), over the pin;
  • T022's one placement: the session runbook, linked from the composed tree's openDox-spec at f7ee3c76 before the run and removed after it. T022's script is on T022, U-3: the runbook placed from the composed tree, git-ignored, nothing vendored (plan 038, P4F-3) #41, not main, and it refuses a checkout whose .gitignore lacks its line. So the run makes that one link itself, with the script's own checks (the source tracked and unmodified, and nothing already at the destination). The checkout read clean after each run;
  • the colour variables unset and TERM=dumb;
  • each file alone, with its own short TMPDIR (38 bytes, under the PostgreSQL socket limit): python -m pytest -q -p no:cacheprovider -rfE --junitxml=… <file> < /dev/null from the checkout root.
tests/test_renderer.py, alone result
base: main fc1115d6 exit 1; 28 passed, 22 failed, 0 errors, 0 skipped
head: 99508076 (main merged) exit 1; 82 passed, 1 failed, 0 errors, 0 skipped
  • Red to green: 21, and 0 green to red. The head's other 33 nodes are the scan's new regression cases, all green. The 21 are exactly the named nodes:
    • DJ + VR: test_model_derives_the_six_columns, test_model_edge_derivation, test_model_tally_equals_edge_count_through_the_real_js, test_five_column_collapse_drops_doc_edges, test_model_degrades_on_empty_snapshot;
    • HH: test_serve_snapshot_route_and_alignment, test_serve_static_bundle, test_serve_source_passthrough, test_serve_divergence_diverged, test_serve_divergence_unknown_without_head, and test_serve_rejects_path_traversal × 7;
    • CP: test_data_fetches_target_only_snapshot_and_source_passthrough, test_the_session_transport_stays_out_of_the_fetch_bearing_set, test_six_column_default_and_five_column_collapse_marker;
    • PI: test_dynamic_import_is_absent.
  • The 3 LI nodes cleared with T095 (T095: the composed harness for the declared integration tests (plan 038, T094's R2-INV-R9) #46), at the base: test_generate_and_open_generates_and_always_prints_url, test_generate_and_open_respects_no_open and test_generated_snapshot_is_schema_shaped were red at 36bbef79 and are green at e2340bb6 and since. They were T095's under R9-W2, not this slice's.
  • Still red: 1, not this slice's. SF, which arrives with T104's openDox pin (doxchat-no-model and doxchat-untrusted have no styles.css rule in the openDox these runs install, openDox-code 84f8ed83): test_every_constructed_doxbench_class_has_a_styles_rule.
  • Every base and head along the way read the same way. The bases 8b64fae0, d45a0939 (T023), f70bc9c3 (T021) and 36bbef79 (T026) read the same 25 failures, and e2340bb6 (T095) read the same 22. Every head of this PR, from f82abd40 to 99508076, read the same 21 flips with 0 green to red.

The unchanged set (the same runs, each file alone)

set base fc1115d6 head 99508076
12.5's 16 governed suites (git grep -l -e open-pr -e open_pr -e FakePullRequests -- 'tests/test_*.py') 842 passed, 0 failed 842 passed, 0 failed
F5.2's 5 (test_generator, test_session_snapshot, test_snapshot_determinism, test_snapshot_registry, test_snapshot_validation_launch) 123 passed, 0 failed 123 passed, 0 failed
tests/test_column_contributions_governed.py 9 passed 9 passed
  • All 22 files read node for node identical at base and head.
  • tests/test_declared_exclusion.py lone, with CI's install (openDox at its pin dede32b4) and no composed tree, at 99508076: 207 passed in 27.51s. Run lone, tests/test_renderer.py still fails on its declared reason, doc_health.

Mutants: 32 of 32 killed

The bundle mutants run over a fresh copy of the installed openDox bundle, which a test-only plugin hands to the suite as its WEB. The plugin is not committed. The control run, with the copy unmutated, read 4 passed.

mutant what goes red
M1: an extra await import("https://cdn.example/x.js") in views/wheel.js test_dynamic_import_is_absent
M2: the ruled projection-column import removed from repo-selector.js test_dynamic_import_is_absent
M3: "./intent-feed.js" made an absolute URL test_dynamic_import_is_absent
M4: a spaced import ("./display.js") added to views/model.js test_dynamic_import_is_absent
M5: /* a note */ const x = import("https://…"), code after a block comment, in app.js test_dynamic_import_is_absent
M6: the registry loader's base moved off the bundle (new URL("https://cdn.example/")) test_dynamic_import_is_absent
M7: the registry loader's import() removed test_dynamic_import_is_absent
M8: an extra fetch( in projection-index.js both fetch-set pins
M9: the old .collapsed-docs spelling restored in styles.css test_six_column_default_and_five_column_collapse_marker
M12: import, a line break, then ("https://…") test_dynamic_import_is_absent
M13: a generator method, *load() { yield import("https://…"); } test_dynamic_import_is_absent
M14: import /* why */ ("https://…") test_dynamic_import_is_absent
M15: a string holding // ahead of a call on the same line test_dynamic_import_is_absent
M16: a regex literal holding a quote and // ahead of a call test_dynamic_import_is_absent
M17: // c CR import("https://…") test_dynamic_import_is_absent
M18: // c U+2028 import("https://…") (the previous scanner missed it) test_dynamic_import_is_absent
M19: [...import("https://…")] (the previous scanner missed it) test_dynamic_import_is_absent
M20: import U+FEFF ("https://…") (the previous scanner missed it) test_dynamic_import_is_absent
M21: a CR LF line continuation in a string, then a URL string and a call test_dynamic_import_is_absent
M31: the real projection loader replaced by a decoy string, 'import("./projection-index.js")' test_dynamic_import_is_absent
M32: the loader's base moved to new URL("../../", import.meta.url), with the old line kept in a comment test_dynamic_import_is_absent
M10: the test's own repo-selector.js count pin relaxed from == to >= (in a copy of this file) test_the_session_transport_stays_out_of_the_fetch_bearing_set
M11: the test's own projection-index.js count pin deleted (in a copy of this file) test_the_session_transport_stays_out_of_the_fetch_bearing_set

The SCANNER mutants (M22 to M30) each run in a temporary copy of this file, and each is killed by a named regression case:

mutant the case that goes red
M22: U+2028 and U+2029 dropped from the line ends u2028-ends-line-comment, u2029-ends-line-comment
M23: a lookbehind that skips any dot spread
M24: U+FEFF dropped ufeff-before-paren
M25: the member-call skip removed member-call-is-not-one, optional-member-call-is-not-one
M26: string state removed slashes-in-string, slashes-in-template, crlf-continuation-in-string
M27: regex-literal state removed escaped-slash-then-slash-in-regex, in-a-regex
M28: the executable reading ignores literals in-a-string, in-a-template, in-a-regex, call-beside-a-decoy
M29: the loader's base ignores literals changed-with-the-old-line-in-a-string
M30: the loader's base reads plain assignments only a-later-compound-assignment

DJ's own mutant is the base: the same harness without the display.js copy reads ERR_MODULE_NOT_FOUND on all five model nodes.

CI at the head

At the head 99508076, every check passed:

  • validate, run 37553249790: CI's own pytest step reads triple: selected=1107 passed=1103 skipped=4 failures=0 errors=0, against pins: selected>=1099 (margin 8) passed>=1095 (margin 8) skipped==4. This is the lone CI form, and it is unchanged: tests/test_renderer.py is a declared file, so the step never collects it.
  • SonarCloud Code Analysis.
  • copilot-pull-request-reviewer.

Copilot

Reviews at each head, all requested through the reviewer API:

head review verdict findings
f82abd40 5433258934 Changes recommended 1: the line scan missed import + newline + ( and generator lines (fixed, 19be4997)
19be4997 5433357731 Changes recommended 1: // ends at CR, U+2028 and U+2029 too (fixed, 8081f9ff, with spread, U+FEFF and CR LF found by audit)
8081f9ff 5435170341 Changes recommended 2: regression tests for the scanner (fixed, ec1486ca); rename test_five_column_collapse_drops_doc_edges (declined, below)
ec1486ca 5435232658 Needs a closer look None
75eb3779 5435680993 Needs a closer look None
a95900c0 5435796042 Changes recommended 2: prove the ruled loaders are executable calls, and read the loader's base from executable code (both fixed, 781c7917)
99508076 5436219525 Changes recommended 1: a shadowing bundleRoot binding at the import site (an ACCEPTED LIMIT, ruled, below)

Threads: 7, all answered and resolved, 0 unresolved.

  • The rename, declined (r4200975189). test_five_column_collapse_drops_doc_edges keeps its name. It is one of the 21 nodes that T094's map and T073's declared run account for BY NAME, so a rename would turn a red-to-green flip into a vanished node and a new one. The name still holds: the edges the collapse drops are the source station's edges, built from the snapshot's document_edges and checked against document_links. Only the column KEY became the role.
  • The accepted limit (r4201878108), RULED by the holder under the convergence rule (5988818366) at openDox + openXdox: two open-source layers, an installable app with users, projects and a database; domain descendants pin openXdox (Brett's ruling 2026-09-04) openxFactory#656 comment 6028383410. The pin is a static, whole-source lexer check, so it cannot resolve scopes. A parameter or inner binding named bundleRoot that shadows the base at the import site would not be seen. That payload needs a structural edit of reviewed openDox code, not a simple one. The head's review therefore still reads "Changes recommended" for this one ruled limit.

Files changed

tests/test_renderer.py, and nothing else.

What is NOT done

  • The 1 node still red: SF, which is T104's. T104 is on openDox-code main (d616da3), and the node clears composed once its openDox pin pair moves.
  • A stale comment in a file that is not this slice's: src/openxdox/web/views/swb-session.js:9 still says test_renderer.py:130 pins the set "to five". The set is six now, and the line number has moved. It is a comment, so no test reads it.
  • validate.yml's floors are untouched. tests/test_renderer.py is a declared file that the lone CI step never collects, and only T029 raises the floors.

🤖 Generated with Claude Code

Summary by Sourcery

Align renderer tests with the current openDox bundle contracts and strengthen their composed boundary checks.

New Features:

  • Add renderer coverage for the bundle’s ruled dynamic imports and their bundle-local loader constraints.

Bug Fixes:

  • Update renderer tests for the split projection index, role-based column names, renamed collapse markers, the model’s display dependency, and duplicate Host-header rejection.

Enhancements:

  • Strengthen fetch-boundary assertions and add lexer-style regression coverage for dynamic-import and loader-base scanning.

Tests:

  • Expand renderer tests to validate dynamic-import scanning across scripts, literals, comments, regexes, line terminators, and executable calls.

…H 12, CP 3, PI 1) (plan 038)

T094's map (R2-INV-R9), under the holder's rulings at openxFactory#656
comment 6021830531:
- DJ (W1 (A'')) + VR: the model harness copies display.js beside model.mjs,
  as #43's harness does, and reads the six columns by S7's role keys.
  No assertion reads a display word, so no served display is needed.
- HH: the raw client sends ONE Host line (openDox-code 390e2c2, T103).
- CP: the fetch-bearing set and counts respelled to S4's split (69d2760,
  RULED Q3); the collapse modifier spelled by role (3c3a9e3, #35).
- PI (W3): test_dynamic_import_is_absent INVERTED: it asserts the ruled
  dynamic imports of contributed bindings, exactly, and nothing else.

Composed (R1Q23 (a)), each file alone, base d45a093: test_renderer.py
25 red -> 4 red (the 21 flip; the 4 left are T095's LI 3 and T104's SF 1).
The 16 governed suites, F5.2's 5 and test_column_contributions_governed.py
read node for node unchanged.

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 6, 2026 18:56
@sourcery-ai

sourcery-ai Bot commented Oct 6, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR updates tests/test_renderer.py for the renderer's current composed contract: it fixes the Node and HTTP harnesses, migrates model and CSS assertions to role-based names, revises fetch-surface counts after binding decomposition, and validates the exact permitted dynamic imports. The stated result is 21 renderer reds cleared with no regressions in the unchanged test sets; four unrelated nodes remain red for later slices.

File-Level Changes

Change Details Files
Updated renderer tests to follow the latest role-based funnel model and its local JavaScript module dependencies.
  • Copy display.js and configure an ES-module package in the Node harness.
  • Rename asserted column keys from product-specific names to role-based names while preserving counts and edge expectations.
  • Use source/grouping terminology for collapse and tally assertions.
tests/test_renderer.py
Aligned HTTP renderer fixtures with the server's strict single-Host requirement.
  • Suppress the HTTP client's automatic Host header and send exactly one explicit loopback Host header.
  • Preserve route, static bundle, passthrough, divergence, and traversal coverage.
tests/test_renderer.py
Rebased fetch-surface and CSS assertions onto the carved renderer bindings.
  • Move the snapshot-index fetch from repo-selector.js to projection-index.js and pin the resulting six-file, per-file fetch counts.
  • Reduce repo-selector.js expectations to its remaining refresh and projection-registration routes.
  • Assert role-based source collapse selectors.
tests/test_renderer.py
Replaced the obsolete blanket dynamic-import prohibition with an exact allowlist for ruled contributed-binding loaders.
  • Scan JavaScript, HTML, and CSS code while ignoring comment-only lines.
  • Require exactly the three approved dynamic-import sites and arguments, plus bundle-root resolution for the registry loader.
  • Retain the existing test name while inverting its assertion semantics.
tests/test_renderer.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The dynamic-import scanner can miss valid JavaScript loaders split across lines.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Updates renderer tests to match the current composed openDox bundle contract.

Changes:

  • Updates fetch, role-key, CSS, and Node harness expectations.
  • Fixes HTTP harness Host handling.
  • Pins the three permitted dynamic imports.
File Description
tests/​test_renderer.py Aligns renderer tests with carved bundle behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/test_renderer.py Outdated
… it (Copilot r4199310756)

The line scan missed two valid JavaScript spellings: a newline between
`import` and `(`, and a generator method whose line opens with `*`, which
it read as a block-comment line. `_without_comments` now removes comments
from the whole source, with string, template and regular-expression literal
state, and keeps those literals, so the scan may over-count but never hides
a call. Mutants M12 to M16 (the two payloads, a comment between `import`
and `(`, and `//` inside a string or a regex literal ahead of a call) are
killed, with M1 to M9 still killed and the unmutated bundle reading exactly
the three ruled sites.

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The dynamic-import scanner can hide imports following non-LF JavaScript line terminators.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread tests/test_renderer.py Outdated
brettheap and others added 2 commits October 6, 2026 22:05
Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… and U+FEFF imports count (Copilot r4199398128)

- A `//` comment now ends at any of LF, CR, U+2028 and U+2029, and a
  regular-expression literal stops at the same four. U+2028 and U+2029 were
  the real gap (M18): `Path.read_text()` already turns CR and CR LF into LF,
  so a bare CR (M17) was a line end before this change too.
- `import` may be followed by U+FEFF, which JavaScript reads as whitespace
  and Python's `\s` does not (M20).
- A spread, `...import(`, is a dynamic import. The lookbehind used to skip
  any `.` before `import`; now only a member call (`a.import(`) is skipped (M19).
- An escape inside a quoted string skips a CR LF line continuation whole (M21).
- The scan also reads `.mjs`, `.cjs` and `.htm` files.

Mutants M1 to M9 and M12 to M21 are killed, and the unmutated bundle reads
exactly the three ruled sites. Run over the same mutated bundles, the
previous scanner missed M18, M19 and M20.

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The new lexer needs committed regression coverage, and one test name remains stale.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
Resolved since last review (1)

Comment thread tests/test_renderer.py Outdated
Comment thread tests/test_renderer.py
…5139)

The scan's branches were exercised only by uncommitted mutants. Now they
are held in the suite:
- `test_the_dynamic_import_scan_reads_code_as_a_lexer_does`: 21 probe
  modules, each with the site map the scan must read. They cover comments,
  a line break or comment before `(`, a generator method's line, each line
  terminator, U+FEFF, a spread, member calls, slashes in a string, a
  template and a regex literal, a line continuation, a call spelled inside
  a string (the scan over-counts, never hides), and argument whitespace.
- `test_the_dynamic_import_scan_reads_every_script_suffix`: which file
  suffixes the scan reads.

Scanner mutants M22 to M27 (U+2028 and U+2029 dropped, a lookbehind that
skips any dot, U+FEFF dropped, the member-call skip removed, string state
removed, regex-literal state removed) are each killed by a named case.

Also in this commit:
- The CR LF escape branch is dropped as dead code: the file is read with
  universal newlines, so a CR LF arrives as one LF. The CR LF case still
  passes end to end.
- U+FEFF, U+2028 and U+2029 are spelled as escapes, never as raw
  characters in the source.

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The dynamic-import scanner can mistake literals for loaders and miss inline imports in HTML.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Ignore strings when detecting dynamic import sites

tests/​test_renderer.py:749

Keeping literal bodies makes the exact allow-list unable to prove that the three required loaders are present in code. If an approved call is removed but the same text remains in an error string (for example, "import('./projection-index.js')"), _dynamic_import_sites still returns the approved site and this test passes; the new spelled-in-a-string-over-counts case explicitly locks in that false positive. Ignore ordinary string/regex contents and scan only executable code (including template substitutions), ideally with a JavaScript parser.

This issue also appears on line 815 of the same file.

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The PR remains draft, CI is pending, and its primary suite is excluded from normal validation.

Review effort: Balanced
Findings: None

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The dynamic-import checks can be satisfied by inert source text and do not reliably enforce the intended loader boundary.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)

Comment thread tests/test_renderer.py
Comment thread tests/test_renderer.py Outdated
brettheap and others added 2 commits October 7, 2026 00:30
…is read from code (Copilot r4201529597, r4201529642)

- `_lex` (was `_without_comments`) also returns, for each character,
  whether it sits inside a string, template or regular-expression literal.
- The pin now reads the bundle twice:
  - the default reading keeps every spelling, so it may over-count, and it
    refuses an unknown import;
  - `executable_only` skips a spelling inside a literal, so it proves each
    ruled loader is an executable call. A decoy string can no longer stand
    in for a removed loader (bundle mutant M31).
- The loader's base is read as every executable assignment to `bundleRoot`,
  plain or compound, each to the end of its statement. It must be exactly
  `bundleRoot = new URL("../", import.meta.url)`, so neither the old line kept
  in a comment or a string nor a later reassignment passes (bundle mutant
  M32).

Regression cases: 6 for the executable reading, 5 for the loader's base.
Scanner mutants M28 to M30 are each killed by a named case, and M1 to M27,
M31 and M32 are killed with the control clean.

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@brettheap
brettheap requested a balanced review from Copilot October 7, 2026 00:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The loader-root assertion does not detect scope shadowing, leaving its external-URL security guarantee incomplete.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)

Comment thread tests/test_renderer.py
@brettheap brettheap changed the title DRAFT: T099: test_renderer.py, the renderer's 21 composed reds (DJ + VR, HH, CP, PI) (plan 038) T099: test_renderer.py, the renderer's 21 composed reds (DJ + VR, HH, CP, PI) (plan 038) Oct 7, 2026
@brettheap
brettheap marked this pull request as ready for review October 7, 2026 00:48
@sourcery-ai

sourcery-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 18 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap requested a balanced review from Copilot October 7, 2026 00:49
@sonarqubecloud

sonarqubecloud Bot commented Oct 7, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The bundle-root security check can miss URL-object mutations and indirect reassignment.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread tests/test_renderer.py
@brettheap

Copy link
Copy Markdown
Contributor Author

READY (holder, manual landing) at 7a3aeb7

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Holder, on Brett's word "land lane 3's PRs when green" (2026-10-06). At 7a3aeb70 the PR is CLEAN, its rollup is SUCCESS, and it has 0 unresolved threads and 0 closing references. Copilot reads "Changes recommended" because of one finding, 4201928341. The holder ruled it an ACCEPTED LIMIT at opensoft/openxFactory#656 comment 6028539440, and the thread cites that ruling and is resolved. The holder lands the PR by the manual path.

@brettheap
brettheap merged commit 4aa3070 into main Oct 7, 2026
4 checks passed
@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

LANDED — lane openxfactory-4, 2026-10-07T00:58:48Z, PR #47 → 4aa3070 (opensoft/openXdox-code main; plain gate)

holder manual landing; Copilot 4201928341 accepted limit at openxFactory#656 6028539440

brettheap added a commit that referenced this pull request Oct 8, 2026
Brings T101 (#48), T098 (#51), T103 (#49), T096 (#53), T100 (#54), T099
(#47), T021's follow-on (#55), T102 (#52), T022 (#41, landed as aefd109),
T097 (#50) and T096's follow-on (#56).

Conflicted: scripts/composed_placements.py and tests/test_composed_placements.py,
both T022's files, which this branch carried from #41 through #44. Main's
landed side is kept for both. tests/declared_exclusion.yaml merged clean
(T101's entry edits beside T073's header and open_until).

Lane: openXfactory-3

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants