Repository navigation
Conversation
added 24 commits
October 23, 2026 17:42
added 5 commits
October 25, 2026 19:09
There was a problem hiding this comment.
✅ Re-review complete · edb2c40
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · 5ee3712
No findings in this incremental review.
lanyue-llk
requested changes
Oct 26, 2026
lanyue-llk
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the current head (5ee3712) against b824b1e0, including the revised acceptance contracts, source/reference checks, body coverage, document structure and shared HTML/table normalization. Compared the linked v7 review with the current code. Focused changed-path tests and catalog validation passed. One public-scoring regression remains (inline): the nested-table safeguard also removes a valid owning link. It needs a narrower ownership rule and paired acceptance controls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Latest reviewer follow-up (2026-10-26)
Accepted and reproduced the owning-link finding. Commit
c1c7ee7retains the outer link URL and its own label when it wraps a nested table, while keeping nested-table text separate. An empty owning label cannot borrow the inner table's text; ordinary links retain their previous behavior. The production change is +7/-2 lines. Four publicscore_attempt/task_passedcontrols cover own/absent labels with/without a nested table; the original code fails the own-label+nested-table control.Final local
make check: 3,130 tests and 349 subtests pass. Both focused table test files pass (202 tests). Reused hash-verified historical inputs to compare 224 HTML-table normalization surfaces against5ee3712; no surface changed, so no historical rescore was repeated. This does not replace the four deferred full exports. Catalog binding now points to scorerc1c7ee7098c00bff3b176dcc5d40ba9b75aa0462; PR head is12cec91. Earlier validation counts and the overview image describe the preceding head. CI for this head passed on Python 3.11–3.14, including source tests, package builds and isolated-wheel validation (run 40326162662). Automated re-review completed with zero findings. The reviewer thread has an evidence-backed reply and is resolved. This follow-up is complete; the PR remains open and unmerged.This revision tightens Web Fetch acceptance where titles, navigation, duplicated links or incomplete article endings could satisfy the existing rules. It preserves frozen task URLs, prompts, reference bindings and saved outputs; changed annotations are selected explicitly with
--revision review-550-20261023.The revision covers 529 reviewed entries across five suites, including evidence-only retentions. It does not certify all 550 frozen references or every historical result. See rule decisions and shared scoring impact.
Rule and implementation changes
Ancillary shared changes
answer_formatpreserves literal plain-text answers, including PDF angle brackets and headings containing&; dedicated regressions cover accepted and rejected forms. This stays with the rule changes because their source checks depend on correct text interpretation.--review-sourceis optional. Original frozen input verification remains mandatory. Independent evidence is hash-checked when supplied; absent evidence is reported as unverified, never as a successful complete check. Captures remain outside Git.Review decisions and limits
Do not remove navigation or reference material solely because it is not article prose. Do not cap every large document at 20 headings: the affected contracts do not have the assumed 80% body-matching fallback. Retain definition descriptions and selected heading elements where source controls prove ordinary heading checks insufficient.
Deferred full export checks
Full frozen-input export checks for
webfetch,popular-sites,dynamic-content, andaccess-restrictionsare deferred at the PR requester’s explicit instruction in the development discussion on 2026-10-26. They are not claimed as passed and are not a closeout requirement for the requested work. This records the requester’s scope decision, not maintainer approval to merge or release without those checks.Reason: the exact hash-matching original input bundles were not found in the local project, evidence directories or searched archives. A fresh capture or a different dataset version cannot validate compatibility with those frozen inputs. Existing unit tests, historical-output scoring comparisons and the articles export provide useful coverage, but do not replace these four end-to-end checks.
Additional rationale and local search (2026-10-26): reconstructing or substituting missing frozen inputs would expand this rule-revision task and would not prove that the original frozen inputs work. Repeating already completed tests would not fill that gap either. The requester therefore chose to defer these four checks for this PR closeout, relying on the validation above while explicitly retaining the limitations below. This is not a claim that these checks have no value or can never reveal problems.
The follow-up search recursively checked the requester-designated local
Documents/Codexdirectory: 12,285 files, including hidden and Git-ignored ordinary files, plus 689 members of 10 archives. None matched the four required dataset SHA-256 values. There were 22 files nameddataset.json, but none was the required version. Git internals, dependency environments and cache directories were excluded. This means the required versions were not found in the searched scope, not that they cannot exist anywhere on the computer.Still unverified for those four suites: whether the real original files can complete the revised export, whether revised rules accept their executable frozen reference answers, and whether the complete exported datasets and declared file bindings are consistent. Export or reference-acceptance incompatibilities may remain. Runtime input-hash and reference checks have not been disabled or weakened to bypass this gap. Repeat the four checks when the exact original bundles become available.
This is a code-and-rule change submitted for review, not an archival-only PR. The requester instructed the agent to leave it open and not merge. Maintainers decide any eventual merge and release requirements; the deferred checks remain an explicit verification gap.
Validation and follow-up corrections
The implementation corrections fall into four groups:
test_normalized_link_cannot_import_nested_table_textcovers accepted own-cell content, rejected borrowed content, preserved inner tables and ordinary links.Local
make checkpasses: 3,126 tests and 349 subtests, including catalog validation. The catalog is bound to the final scoring source; no raw captures or run-local evidence are committed.An additional 64-combination check covers both caption-expansion and complete-column paths, crossing header cell styles, overlapping record selectors, nested-table interference, correct/wrong labels and present/missing required values; all match their expected verdicts. It reuses synthetic inputs and does not replace deferred frozen exports.
The source comparison covers 65 precisely hash-bound section inputs with no changed verdicts. Three old projection records lacked exact matching bindings and are excluded, not claimed as passed. Rechecking existing article sources qualifies 17 full tasks, 75 body-only controls and three body-plus-added-stable-text controls; all 95 changed-rule controls pass, but whole-task completeness remains false. All 4,820 archived outputs were rescored with
03d23e7; input hashes match the saved records. There are no additional pass/fail changes from the preceding comparison: 1,646 pass, retaining the same 18 differences from baseline. The three historical table/linked-table cases contain 30 outputs, without rendered HTML tables; dedicated controls establish nested/thead behavior. Final scorer19ba48areuses that full replay with targeted validation: 214 normalized HTML-table answers were rescored after the heading correction; a final comparison checked 224 table-tag-prefix answers and rescored the 101 with changed normalized text. No verdict changed. All reuse checks artifact, result-metadata and contract hashes; this is not another full 4,820-output scoring run. Four complete frozen exports remain deferred as above.Current code is bound by catalog scorer commit
19ba48a06c9de449b5a7e9b8d351b9f8cd5a042band its scoring-source hash. After a squash merge, a follow-up PR must replace the locator with the full merged commit and verify the unchanged hash before publishingbench-v2.1.0. Reverting to main's old catalog would mismatch the new scorer and fail validation. No release tag is created and this PR stays unmerged.Review decisions and Lexmount limitation
The PR title follows CONTRIBUTING. Repository docs retain reusable decisions and evidence limits, while reviewer dialogue and machine-specific IP details have been removed. A revision-specific test of the historical 104-case prose inventory is possible; its frozen membership was already checked, so no redundant prose-table test was added. Existing rule controls remain intact.
Saved Lexmount Moli evidence records private-address rejection with its native safeguard enabled. Subsequent UDP and HTTPS/TCP DNS answers differed; the responsible device or software is unknown, and public resolution did not fix local connection failures. An independent hosted Firecrawl request (
maxAge=0) returned HTTP 200 and 6,202 characters passing the rule. This is diagnostic evidence only, not a replacement Moli result or proof that Moli works elsewhere. Original case, failed measurement, safety checks and network settings are unchanged; no target-specific exception was added.The earlier OCR HTTP 403 does not establish the suggested fork-secret cause: the run used
pull_request_target, and dispatch subsequently succeeded without configuration changes. The resulting parser findings are addressed above; the export-policy disagreement is explained below. CI for head5ee37128b898cb34b18619d472603d9c94628c65passed on Python 3.11–3.14, including the full tests, catalog validation, package builds and isolated-wheel tests (run 40309478225). The latest automated review also passed. Local checks and historical validation are recorded above. Requested changes are closed out; this PR remains open and unmerged.V6 review disposition
The saved-output rescore is accepted because the preceding comparison used an older shared scorer; section-only controls cannot establish table compatibility. The source and result counts are recorded in
docs/data.md. The linked-inner-table finding was reproduced and corrected with one ownership guard and accepted/rejected controls. Lexmount repository prose is shortened to the task/rule decision and limits; diagnostic observations remain above and captures stay outside Git.All 33 review threads covered by v6 have received an individual evidence-backed reply and are resolved, including the two explicit non-adoptions. One subsequent reproduced regression is also corrected: visible headings were extracted before another HTML serialization pass. Exclusions and conversion now share one parsed document, followed by heading extraction after serialization, even when all HTML tables are excluded. This preserves scope labels without character-specific exceptions. Six heading controls cover ordinary/mixed tables, accepted local rows, rejected out-of-scope rows and missing headings; full local checks pass. Its thread is also replied to and resolved. Existing findings are assessed against the v6 review and implementation evidence. A resolution records either the identified fix or an explicit reasoned non-adoption, not blanket automated-review approval. Further suggestions are evaluated on evidence and scope, not accepted automatically.
The four full exports remain deferred at the requester's instruction, not waived for release. Maintainers must decide their release requirements and approve the post-squash scorer-binding workflow before publishing. No maintainer approval is claimed, no release tag is created, and this PR is left open and unmerged. The two evidence-backed non-adoptions below remain unchanged.
Non-adopted automated-review proposal
The export-status finding correctly describes an intentional distinction, but its proposed blanket export failure is not adopted. Export may materialize an explicitly unverified dataset; success is not a reference-acceptance certificate. With an omitted registered alternative, failed evaluated candidates remain visible, status is unavailable, and completion flags are false. The CLI separately reports
unverified_with_failed_frozen_reference; original input-byte checks still run. Without an omitted alternative, all failing executable candidates reject export. An accepted negative/error control rejects export even when evidence is omitted. Supplied evidence must match its registered hashes.Evidence:
ReferenceControls.test_reference_checks_reject_missing_required_content_and_report_missing_sourcesandtest_missing_reference_cannot_hide_an_accepted_error_pageexercise these distinctions. The exact frozen article-072 body fails page identity and the same 19 stable anchors under both original and revised rules while body coverage passes. Thus that incomplete candidate cannot establish a correction-induced regression. With independent evidence supplied, all 95 changed-article controls pass, while overall completeness still remains false. The tradeoff is explicit: an unverified export can still contain a rule/reference incompatibility and must not be presented as qualified. Requiring optional captures for every materialization would contradict the accepted optional-evidence workflow rather than fix a hidden pass.Blanket exclusion of nested definition lists is not adopted. The contract checks local structural ownership, not semantic relevance: an immediate
ddcan legitimately contain a parameter-definition list. The HTML standard permits flow content indd, includingdl.test_nested_parameter_list_is_local_descriptionaccepts a parameter explanation inside the owningddand rejects the same list outside it. Excluding all nested entries would reject that legitimate input. The exact bound os.path source contains no nesteddd dl; no historical nested-source success is claimed. A nested unrelated API can still satisfy this structural floor, a disclosed semantic limitation rather than evidence that all nested descriptions should be deleted.