Skip to content

feat(tp4): analyze fenced code in Markdown as skill implementation - #421

Open
rodboev wants to merge 3 commits into
NVIDIA:mainfrom
rodboev:pr/tp4-markdown-fences-419
Open

feat(tp4): analyze fenced code in Markdown as skill implementation#421
rodboev wants to merge 3 commits into
NVIDIA:mainfrom
rodboev:pr/tp4-markdown-fences-419

Conversation

@rodboev

@rodboev rodboev commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

TP4 now inspects bounded executable-language fences in Markdown/text reference files, after ordinary executable components. This closes the Markdown-only implementation blind spot while preserving existing limits and telemetry.

Root cause

TP4 built its candidate list only from components whose metadata type was one of the executable types. Markdown reference files were therefore absent from the LLM prompt, with no TP4 call or ledger evidence that implementation content had been skipped.

Changes

  • Parse only backtick or tilde fences with normalized executable-language labels, including common aliases such as bash, sh, py, and js.
  • Reject unlabeled, unsupported, output/data, and unterminated fences.
  • Aggregate accepted fences from each Markdown file into one bounded candidate after executable candidates, preserving each fence's source label in the prompt.
  • Record Markdown byte overflow as a partial ledger event, while a clean code-free input makes no TP4 call and retains its existing status.

Preserved

Existing executable ordering, TP4 limits, deadline, prompt safety rule, finding schema, report scoring, and non-TP4 analyzers remain unchanged.

Testing

Focused TP4 tests cover the issue scenario, accepted aliases and rejected fences, the 32768-byte boundary and overflow ledger event, one-candidate aggregation, ordering, telemetry modes, and clean status preservation.

Scope

Closes #419. No live provider call is required for the focused tests.

@benedictkwok

Copy link
Copy Markdown
Contributor

Confirming this PR addresses my reported scenario. Thanks for picking this up. Checked the diff against the two open questions I flagged in #419:

  • Runnable-vs-illustrative fences: the exact-label requirement (rejecting unlabeled/unterminated/non-executable-labeled fences) is a reasonable, conservative proxy — matches the real case in pdf's SKILL.md where fenced Python is the actual delivery mechanism.
  • Token-budget impact: TP4_MAX_MARKDOWN_BYTES as a separate limit from TP4_MAX_FILE_CODE_BYTES, with overflow recorded as a partial ledger event, is exactly what I had in mind.

Test fixture using a fenced requests.post(...) exfil against a manifest claiming "never accesses the network" is a good concrete repro of the original problem. Looks solid to me.

@yashrajp22
yashrajp22 self-requested a review August 24, 2026 09:14

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[SkillSpector Review]\n\nApproved. TP4 now inspects bounded, executable-labeled Markdown fences after ordinary executable components, preserves source labels and ordering, and records conservative partial coverage on Markdown overflow. The clean/unsupported-fence and limit telemetry cases are covered, and current required checks pass.

@mohgupta-ship-it

mohgupta-ship-it commented Aug 25, 2026

Copy link
Copy Markdown
Member

Powered by Codex: PR council review result.

This is a triage signal, not a maintainer approval.

  • Rating: new feature
  • Confidence: high
  • Status read: Approved, branch behind, green checks
  • Review method: fresh GitHub metadata/body/files/reviews/checks plus selected diffs; council lenses were spec fit, dead-code/reachability, YAGNI/scope, design/coupling, and code standards/tests.
  • Council assessment: Bounded TP4 Markdown fenced-code analysis with coverage for source paths, ordering, overflow, and executable language aliases.
  • Recommended action: Update branch, rerun CI, then merge if still green.

Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
@rodboev
rodboev force-pushed the pr/tp4-markdown-fences-419 branch from 232605a to e970863 Compare August 25, 2026 12:12
@rodboev

rodboev commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and force-pushed as e970863. The rebased commits retain Signed-off-by trailers, and the new DCO Check is passing. Local focused validation passed: pytest tests/test_mcp_tool_poisoning.py -q (56 passed, 6 deselected), ruff check src/ tests/, and ruff format --check src/ tests/. The remaining hosted checks are running.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants