Repository navigation
fix(embedding): preserve multiline descriptions in catalogs - #191
Merged
chrisknvidia merged 1 commit intoOct 12, 2026
Merged
Conversation
Signed-off-by: Rio Yu <[email protected]>
rioyu123
requested review from
chrisknvidia,
mosheabr,
rmalani-nv and
rng1995
as code owners
October 9, 2026 11:18
chrisknvidia
approved these changes
Oct 12, 2026
chrisknvidia
left a comment
chrisknvidia
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for this PR, Looks good to me!
rng1995
added a commit
that referenced
this pull request
Oct 14, 2026
Brings in #184, #187 (SkillSpector version in the HTML report), #188 (GPT-6.1 Sol / Claude Opus 5.5 defaults, reasoning effort, structured output, token sizing, refusal fallback), #191 (multiline catalog descriptions), #193 (Docker agent bootstrap) and #199 (per-step token runtime-failure detection). Conflicts: - .gitleaks.toml: keep every allowlist from both sides (the PR's three history allowlists and main's Bedrock model ID allowlist). - CHANGELOG.md: every entry from both sides under Unreleased, one Added, Changed, Fixed and Security heading each; the shared docs entry once. - deduplication/intra_skill/intra_skill_validator.py: the client keeps the PR's LLM_VERIFY_MAX_TOKENS and gains main's medium reasoning effort. - deduplication/intra_skill/llm_analyzer.py: the PR's bounded prompt and verdict parsing, with main's structured-output schema request. - embedding/registry.py: keep the PR's _embed_texts refactor (main only reformatted the old loop); main's multiline-description check merged. - inference/__init__.py: export both sides' names. - reporting/html.py: the PR's run-status tables plus main's SkillSpector version helper; is_tier2_validator_name stays in run_status.py. - tier3/eval_core/llm_judge.py: drop _is_native_openai_chat_url (removed by main) and _configured_secret_values (moved by the PR). - tier3/harbor/collector.py: keep the PR's shared-metric pass lift and main's judge-model helpers; _compute_lift takes main's per-trial rewards and the PR's keyword-only N/A lists, and its overall uses the paired scores with the PR's N/A-aware basket check; lift.json keeps the PR's _paired_lift. - tier3/harbor/local_agents.py: the PR's SkillEvaluatorCodex (subagent rollouts, setup network errors) now extends main's bootstrap-aware installed_agents.SkillEvaluatorCodex; NVIDIA Build Claude Code keeps SetupNetworkErrorAgent on main's SkillEvaluatorClaudeCode. - tier3/harbor/runner.py: imports from both sides; Docker uses main's DOCKER_AGENT_IMPORT_PATHS, other Harbor backends the PR's HARBOR_AGENT_IMPORT_PATHS; agent kwargs from main's _harbor_agent_kwargs. - tier3/harbor/templates/eval.py: the PR's skill-metric N/A helpers and allow_not_applicable judges, with main's 16,384-token judge ceiling and judge_model_used / reasoning effort / fallback provenance. - utils/tool_runner.py: main's pinned SkillSpector v2.12.0 hint plus the PR's scanner hints. - validators/security.py: imports from both sides. - tests/inference/test_llm_client.py: keep both sides' tests. - tests/test_harbor_local_mode.py: main's parametrized Docker route check plus the PR's Codex subclass assertion. - tests/tier3/test_judge_parse_robustness.py: the PR's partial-salvage assertions plus main's new provenance and refusal tests. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Narendran Raghavan <[email protected]>
chrisknvidia
pushed a commit
that referenced
this pull request
Oct 30, 2026
Brings in #184, #187 (SkillSpector version in the HTML report), #188 (GPT-6.1 Sol / Claude Opus 5.5 defaults, reasoning effort, structured output, token sizing, refusal fallback), #191 (multiline catalog descriptions), #193 (Docker agent bootstrap) and #199 (per-step token runtime-failure detection). Conflicts: - .gitleaks.toml: keep every allowlist from both sides (the PR's three history allowlists and main's Bedrock model ID allowlist). - CHANGELOG.md: every entry from both sides under Unreleased, one Added, Changed, Fixed and Security heading each; the shared docs entry once. - deduplication/intra_skill/intra_skill_validator.py: the client keeps the PR's LLM_VERIFY_MAX_TOKENS and gains main's medium reasoning effort. - deduplication/intra_skill/llm_analyzer.py: the PR's bounded prompt and verdict parsing, with main's structured-output schema request. - embedding/registry.py: keep the PR's _embed_texts refactor (main only reformatted the old loop); main's multiline-description check merged. - inference/__init__.py: export both sides' names. - reporting/html.py: the PR's run-status tables plus main's SkillSpector version helper; is_tier2_validator_name stays in run_status.py. - tier3/eval_core/llm_judge.py: drop _is_native_openai_chat_url (removed by main) and _configured_secret_values (moved by the PR). - tier3/harbor/collector.py: keep the PR's shared-metric pass lift and main's judge-model helpers; _compute_lift takes main's per-trial rewards and the PR's keyword-only N/A lists, and its overall uses the paired scores with the PR's N/A-aware basket check; lift.json keeps the PR's _paired_lift. - tier3/harbor/local_agents.py: the PR's SkillEvaluatorCodex (subagent rollouts, setup network errors) now extends main's bootstrap-aware installed_agents.SkillEvaluatorCodex; NVIDIA Build Claude Code keeps SetupNetworkErrorAgent on main's SkillEvaluatorClaudeCode. - tier3/harbor/runner.py: imports from both sides; Docker uses main's DOCKER_AGENT_IMPORT_PATHS, other Harbor backends the PR's HARBOR_AGENT_IMPORT_PATHS; agent kwargs from main's _harbor_agent_kwargs. - tier3/harbor/templates/eval.py: the PR's skill-metric N/A helpers and allow_not_applicable judges, with main's 16,384-token judge ceiling and judge_model_used / reasoning effort / fallback provenance. - utils/tool_runner.py: main's pinned SkillSpector v2.12.0 hint plus the PR's scanner hints. - validators/security.py: imports from both sides. - tests/inference/test_llm_client.py: keep both sides' tests. - tests/test_harbor_local_mode.py: main's parametrized Docker route check plus the PR's Codex subclass assertion. - tests/tier3/test_judge_parse_robustness.py: the PR's partial-salvage assertions plus main's new provenance and refusal tests. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Narendran Raghavan <[email protected]>
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.
Summary
Valid multiline skill descriptions can be embedded successfully but fail when the Tier 2 catalog is saved: catalog validation rejects their line breaks as control characters.
This change allows LF, CR and tabs in catalog descriptions without changing the extracted text. Names, IDs, paths and other metadata keep their existing validation; other control characters, format characters and surrogates are still rejected. Catalog structure and embedding behavior are unchanged.
The tests follow a real
SKILL.mdthrough extraction, indexing, save/load and a similarity query, with a fixed embedding client. They also cover whitespace preservation and continued rejection of unsafe text. One existing long line was wrapped by Ruff.Verification
make lintmake testmake buildSeven regression cases fail on unchanged main; the other 12 are controls that already pass. The embedding suite passes on Linux (243 passed, 2 skipped) and Windows (220 passed, 25 skipped).
The final Linux
make testrun, using an isolated temporary directory, reports 11,237 passed, 51 skipped, 9 deselected and 2 failed. Both failures also occur on unchanged main: a runtime-discovery test finds the host's/usr/bin/codex, and a bridge test needs port 18080, which is already occupied. Neither involves catalog validation.make lint, scoped Ruff formatting andmake buildpass.AI assistance: implemented with Codex and reviewed with Claude.
Release Impact
CHANGELOG.md