fix: avoid false analysis limits in documentation - #516
Conversation
Distinguish Markdown code delimiters from shell substitutions during coverage checks, and reject JSON value closers as fallback removal-marker openers. Add documentation regressions and preserve shell reconstruction coverage across scripts and Markdown code blocks. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Keep list-relative indentation and HTML block terminators when identifying Markdown delimiters. Exercise incomplete-result gates with and without semantic analyzers. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Recognize raw HTML tags at line endings and close fenced list code using its container indentation. Add positive and negative boundary regressions. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Preserve candidate spans while caching quote ends and checking the artifact deadline throughout scanning. Add deterministic deadline and reconstruction compatibility regressions. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Verify bounded work for shared quote suffixes and exercise real graph/MCP findings and completeness with semantic analysis enabled and disabled. Structured model transports are mocked for repeatable CI. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
fix: make JSON quote candidate scanning linear lgtm
Build a fresh graph after mocking LLM availability so credential-free runs exercise all five failing analyzer stages. Preserve every telemetry, report and SARIF assertion. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Recognize structural quotes in bounded, validated JSON arrays, objects, scalar strings and explicit JSON fences, including container prefixes and frontmatter bodies. Preserve source offsets and linear, cancellable scanning. Keep inline delimiter pairing within Markdown blocks so separate list items and headings cannot hide unresolved runtime commands. Retain genuine command findings, incomplete coverage and existing advisory risk policies. Add tests-first reviewer reproductions, neighboring JSON and Markdown controls, linear-work/deadline checks and inline Make reference regressions. Exercise CLI and MCP with deterministic enabled/disabled LLM transports. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
…VIDIA#524 Raise reviewed_pr_through to 527 and reviewed_issue_through to 524 in tools/upstream_baseline.json (commit axis unchanged at 69dcdfb). Every item gets a verdict in docs/DECISIONS.md: NVIDIA#493/NVIDIA#507/NVIDIA#508/NVIDIA#511 verified via git merge-base --is-ancestor as already included through the 2.11.1/2.11.2 sync (including NVIDIA#521, which merged only into the still- open NVIDIA#516 stack, not main); the remaining 27 items stay "wait for upstream merge", none adopted now. Two items get dedicated comparison notes per docs/DIVERGENCE.md's static_runner.py and scripts/compare_scan_accuracy.py rows: NVIDIA#522 uses a different env var name and different default/semantics than this fork's SKILLSPECTOR_MAX_STATIC_SECONDS, so merging it cannot simply delete the divergence row and needs a downstream env var migration first; NVIDIA#490 extends this fork's own upstream PR NVIDIA#486 with a Python 3.14/POSIX edge case the fork's Windows environment does not hit, so NVIDIA#486 is left untouched pending upstream's own resolution. NVIDIA#501-NVIDIA#505 and NVIDIA#518 are also flagged as near-verbatim matches to this fork's existing Windows test divergence rows, worth revisiting for row deletion once merged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: SanHsien <34234698+SanHsien@users.noreply.github.com>
Add tests-first reproductions for table row and cell boundaries, tab-column JSON containers, hidden runtime commands and bounded owned-string recovery. Preserve benign documentation, raw findings, strict CLI and MCP coverage gates in enabled and disabled LLM transport modes. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Add tests-first cases for Setext-like table rows and headers, real block interruptions and strict CLI/MCP gates in enabled and disabled LLM transport modes. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Retain complete and installable benign table exits in both semantic modes, and reproduce empty-list items incorrectly opening a paragraph before the next table header. Preserve red evidence before correcting that state transition. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
Keep inline delimiters within proven table cells and respect contextual header, row and block boundaries. Interpret JSON container indentation in visual columns while preserving raw source offsets. Recover skipped runtime JSON strings through disjoint projected spans without overlapping suffix parses or changing raw finding evidence. Validated against tests-first regressions, documentation and JSON overlap tests, GFM reference cases, bounded-work checks and enabled/disabled CLI/MCP transport modes. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta <mohgupta@nvidia.com>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Reviewed current head bce6afadb2819d2235510e183eaa3a197ac02463 — APPROVE.
I reviewed the post-merge source diff, including bounded JSON ownership, Markdown block/table delimiter ownership, source-offset restoration, runtime checks, and the public completeness gates. The implementation remains conservative when context is incomplete, keeps command bodies analyzable, bounds validation and projection work, and has extensive success/failure/boundary coverage. I found no required changes.
The head changed during review from ca0ceb77e252c0ad5dc84eb435323edb80d28c57 to this merge commit, so the earlier assessment was discarded and the current diff was re-reviewed. Merge gate: GitHub reports mergeStateStatus=BLOCKED and no checks yet on the new head; required lint, unit, and DCO checks must pass before merging.
|
Thanks everyone for code reviews. As I have got two approvals on this change. I wll merge it post the pipeline passes. |
Valid Markdown code spans and JSON request placeholders could produce false incomplete-analysis/AE1 results. Delimiters could also pair across unrelated Markdown blocks or table cells and hide unresolved runtime commands. This change establishes delimiter ownership from complete document context, preserving code bodies and original source coordinates.
Completeness and risk remain separate: existing findings and thresholds are unchanged. Benign documentation can complete, while unresolved static inspection remains incomplete and fails strict CLI/MCP installation gates even when semantic analysis succeeds.
Validation on frozen source
ca0ceb77e252c0ad5dc84eb435323edb80d28c57:make test-ci: 4,793 passed, 14 skipped, 38 provider/integration tests deselected and four expected failures; 89% coverage.codex_clireplay: 40 fixtures in both modes, 80 CLI scans passing their expected completeness, evidence and exact strict/risk-threshold exit contracts. Enabled scans recorded 150 successful analyzer-level attempts out of 150, with all three semantic analyzers completed and no degraded scans. Disabled scans recorded zero attempts.mcp_server.run_scandirectly; it does not certify HTTP/stdio wire transport.--no-llm. All 97 installed Python source hashes match the frozen revision. Live-provider replay ran separately on the host using that same verified source.The targeted and independent runs overlap the full suite; their counts are not additional unique tests. Provider telemetry counts analyzer-level attempts, not individual underlying model requests. These results supersede the earlier validation summary for
b5c5d8e. A separate pre-existing legacy-backtick interpretation gap is not claimed fixed.All five hosted CI checks pass on published
ca0ceb77e252c0ad5dc84eb435323edb80d28c57: changes, lint, test-unit, DCO and docker-smoke (CI run). The Linux full suite independently matches the local result: 4,793 passed, 14 skipped, 38 deselected, four expected failures and 89% coverage.Fixes #515.
Prepared by Codex on behalf of Mohit Gupta.