Skip to content

Dedup KB skill rulebook: extract shared rules into kb-editing-conventions.md - #1370

Merged
hilram7 merged 20 commits into
devfrom
kb-skills-dedup
Aug 12, 2026
Merged

Dedup KB skill rulebook: extract shared rules into kb-editing-conventions.md#1370
hilram7 merged 20 commits into
devfrom
kb-skills-dedup

Conversation

@hilram7

@hilram7hilram7 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR #1358 added kb-writer/kb-pr-open and went through ~15 review rounds, nearly all of which found the same rule (Derek's areas table, title-format rules, callout severity, etc.) duplicated independently across up to five files — fixing one copy never propagated to its siblings. This PR does the structural fix: it extends the existing .claude/references/kb-editing-conventions.md (which already served this role for the scan table and cross-section table) to also cover frontmatter, article-type identification, article structure, title format, callout severity, links, bolding/inline code, and images — and points derek, kb-pr-open, and kb-pr-review at it instead of inlining their own copies.

Changes

  • Extends existing rulebook sections rather than adding parallel new ones where a home already existed: §5 Callout severity (full 7-type→2-severity mapping), §6 Bolding (backtick/registry-path/UI-action-target rules), §8 Links (three valid forms, pathname:// constraint, BoilerplateCrossRef dead-rule note), §12 Titles (full mechanical/semantic rules, both Error: exemptions)
  • Adds new rulebook sections for topics with no existing home: §13 Frontmatter, §14 Article Type Identification, §15 Article Structure, §16 Images
  • Rewrites derek/SKILL.md, kb-pr-open/SKILL.md, kb-pr-review/SKILL.md to read from the shared rulebook instead of inlining. /derek's scope grows slightly beyond a pure copy-move: it now also covers images: external-refs and images: alt-text (both pure single-file text checks), while images: location stays with kb-pr-open/kb-pr-review since it requires checking the actual folder structure on disk beyond the article itself.
  • Reconciles the knowledge_article_id policy to kb-pr-open's version (never propose adding/removing the field; flag malformed values only) — kb-pr-review previously allowed proposing removal of a placeholder value, contradicting kb-pr-open
  • Fixes kb-pr-review's stale BoilerplateCrossRef note — the Vale rule is unreachable on docs/kb/**/*.md files (confirmed empirically), kb-pr-open already knew this
  • Syncs wording drift discovered while extracting: kb-pr-review's scan-table rows 8/13/14/15 and its cross-section consistency table had drifted from kb-pr-open's wording; both now match exactly

Behavior changes (not pure refactor)

Pointing /derek at the shared rulebook changes its standalone output beyond deduplication:

  • knowledge_article_id missing entirely now prints a soft-reminder note. Pre-PR, standalone /derek said "if absent, do not flag" — no output at all. Post-PR it inherits kb-pr-open's already-established policy (a non-counted soft reminder). Affects 47 of 1467 KB files.
  • products field validation tightens from "present, contains at least one product ID" to an exact src/config/products.js match (plus the onesecure/general/recoveryad carve-outs). Standalone /derek will newly flag the ~300 files using the legacy hyphenated form (already-tracked migration debt, not new debt).
  • Product-name-in-title is downgraded from a counted finding to an uncounted soft-reminder note. Pre-PR, /derek's title check flagged this as an ordinary table row (counted in the issue total). Post-PR it's a semantic reframe per §12 — surfaced as a note with suggested alternatives, never counted. An article that previously reported one issue may now report zero issues plus a note.
  • Callout indentation and bolding-consistency rules are newly in Derek's scope. §5's 4-space callout-indent rule (previously only kb-pr-open/kb-pr-review scan row 1) and §6's intra-file bolding-consistency rule are new territory for standalone /derek, which will now emit rows for them.
  • Article structure and title/H1-match checks tighten. §15 adds the Q&A question-format check (interrogative + ?, second person), the How-To structure check (H1 + unstructured prose → Required fix), and the wrong-shape check (Q&A form with multiple procedures → Required fix) to standalone /derek, none of which base Derek's §2 (headings + pluralization only) covered. §13's title row also now requires the frontmatter title to match the H1 exactly, where base Derek only checked presence/quoting.

All of these are the intended effect of having one source of truth rather than three, but are worth calling out since /derek's reviewer-facing output changes as a result — expect new (correct) findings on previously-clean legacy articles.

Testing

  • Read all three rewritten skill files end-to-end plus the rulebook for self-consistency; verified every §N cross-reference resolves to a real section.
  • No functional/build changes — this PR only touches .claude/skills/*/SKILL.md, .claude/references/kb-editing-conventions.md, and one CLAUDE.md table row.

…ions.md
derek, kb-pr-open, and kb-pr-review each independently inlined Derek's
areas table, title-format rules, and the callout-severity mapping,
requiring N synchronized manual edits per rule change (root cause of
most add-kb-skills PR #1358 review rounds). Extend the existing
kb-editing-conventions.md sections (callouts, bolding, links, titles)
and add new ones (frontmatter, article-type identification, article
structure, images) as the single source; all three skills now read
from it instead of duplicating.
Also reconciles the knowledge_article_id policy (kb-pr-review allowed
proposing removal of a placeholder value; kb-pr-open never did) to
kb-pr-open's never-remove policy, fixes kb-pr-review's stale
BoilerplateCrossRef note (the Vale rule is unreachable on docs/kb/
files), and syncs wording drift in the kb-editing-conventions scan
table and cross-section consistency table between the two PR skills.
Addresses netwrix-corp/docs-kb-staging#131.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
…e-guide rules
- docusaurus.config.js only relaxes onBrokenLinks under DOCS_PRODUCT;
onBrokenMarkdownLinks and onBrokenAnchors throw unconditionally.
Rulebook §8 overstated this as "relaxes all three to warn".
- §12 Titles referenced Error: prefix exemptions without ever stating
the base Error: prefix rule, and dropped the Resolution (Symptom)
title-format rule entirely during extraction. Added a "Base title
format by article type" subsection covering all four types.
- Restored the kb_style_guide.md read instruction in kb-pr-open and
kb-pr-review — both still depend on it (product-names, images
alt-text) but dropped the explicit read step when switching to the
rulebook as primary source.
- Derek's knowledge_article_id missing-field soft reminder (inherited
from kb-pr-open's already-established policy via the reconciliation)
had no note text in Derek's Output section. Added it alongside the
existing blank-value and placeholder note text.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- Restored the dropped "multi-line commands/command output use fenced
code blocks" rule (rulebook §10); repointed kb-pr-open/kb-pr-review's
formatting: lists area at it alongside §3.
- Derek now inherits a title-case check via §12 that overlaps Vale's
NetwrixKB.HeadingCase (scope: heading, fires on the H1) - noted in
§12 that this is Vale's finding, not a second Derek row, for any
skill running both in the same report.
- Qualified §12's "must not contain a product name" line - it's a
general rule, not absolute; the semantic-reframes subsection below
it is what actually governs (soft reminder, never auto-applied).
- Removed Derek's standalone Keywords/Description section, which had
already drifted from §13's carve-out for plausible-but-unused
keywords; §13 Frontmatter already covers both fields.
- Noted in Derek's Links area that WeakLinkText/BoilerplateCrossRef in
§8 are Vale-only and not Derek's job.
- Added a rulebook-vs-kb_style_guide.md precedence statement: the
rulebook is a derived elaboration, not a replacement - divergence
from the published style guide is a bug in the rulebook to fix, not
grounds for the rulebook to win.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- Dropped the Links and Images rows the extraction had silently added
to Derek's areas table. Derek's Overview and description frontmatter
never claimed those areas, and its single-file "read from disk" flow
doesn't fit link-target/image-location resolution across the repo -
that's kb-pr-open's and kb-pr-review's job. Restored Derek to its
original five-area scope; added an explicit note pointing links/
images checks at the other two skills.
- §13's title/H1-match rule now covers the no-H1 case (13 KB files
have none) - Docusaurus renders frontmatter title as the H1 when the
body has none, so that's a valid state, not a mismatch. Updated
Derek's line-number guidance to fall back to line 1 when there's no
H1 to point at.
- §8's build-backstop note now says pathname:// links have no build
backstop at all (full or single-product) - pathname:// is
Docusaurus's documented escape hatch that bypasses the broken-link
checker entirely, unlike /docs/... links which are at least caught
by onBrokenMarkdownLinks/onBrokenAnchors on a full build.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
…c them
The 23-row scan table and 6-row cross-section table were still fully
duplicated verbatim between kb-pr-open and kb-pr-review, and now
tripled a few rules against the rulebook (§5's 4-space indent vs. scan
row 1, §10's table-cell periods vs. scan row 5) - the exact
whack-a-mole pattern this PR exists to close out, just one level
deeper than the areas table this PR already fixed.
Condensed both tables' "what to flag/fix" column to the pattern
description plus a rulebook-section pointer, dropping the duplicated
rule-mechanics prose while keeping every kb-writer routing decision
(rows 13/17/18/20) that isn't in the rulebook. Row count is unchanged
(23 scan rows, 6 cross-section rows) so the N/N-scanned coverage
mechanism is untouched. Verified the tables are still byte-identical
between the two skills afterward. Also names explicitly that the
cross-section table's rows 1-2 are the two failure directions of one
rulebook §11 bullet, not two separate rules - the exact 6-vs-5 drift
example from issue #131's history.
Also: removed kb-pr-open's now-fully-redundant knowledge_article_id/
title-format pointer paragraph (duplicated the areas table's §12/§13
rows, sitting in a dead spot after the all-clean short-circuit), and
added keyword/description quality to Derek's frontmatter area
parenthetical so its scope description matches the PR skills'.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
… legibility nit
- derek/SKILL.md's scope sentence (added by this PR) contradicted its
own areas table two lines above, explicitly excluding callouts (§5)
and bolding/path formatting (§6) that the table sends Derek to. A
literal regression: those were unambiguously in scope pre-PR as
Derek's own §5 Path Formatting and §7 Admonition Format sections.
Fixed the Overview line, frontmatter description, and scope sentence
to all state the same six-area scope.
- §13's keywords row lost the "flag terms prominent in the article
body that are missing from keywords" check during extraction - both
source files had it, the consolidated row only kept the inverse
direction (keyword present but unused in body). Since this drives
KB search discoverability, restored it as a Required fix distinct
from the softer inverse-direction observation.
- §12's Vale-deferral for title case fires unconditionally, but Vale's
NetwrixKB.HeadingCase is heading-scoped and never evaluates the 13
KB files with no body H1 (where Docusaurus renders frontmatter title
as the H1 instead). Scoped the deferral to H1-bearing articles only;
no-H1 articles now get the frontmatter title's case checked directly
since nothing else will.
- Promoted the Error: prefix's two exemptions to a top-level sub-bullet
next to the Resolution (Error) rule they modify, instead of nesting
exemption (a) - a general rule unrelated to log-dump normalization -
under the narrower "raw log dump" bullet where a model evaluating
that bullet as inapplicable would never reach it.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- Carved out §12's product-name-in-title and §13's no-H1-is-valid
rules from the new rulebook-vs-style-guide precedence statement -
both read as literal disagreements with kb_style_guide.md's absolute
wording, but are deliberate elaborations, not bugs to revert.
- Fixed derek/SKILL.md's area count: Product Names lives in its own
section below the table, not as a table row, so "six-area scope"
underclaimed by one against the Overview's seven-area list.
- Restored the no-target-found branch for <!-- link removed --> (leave
the comment, note unresolved) - dropped in the scan-table condensing
pass, in both skills and the rulebook.
- Scan row 4 (intra-file bolding consistency) pointed at §6, but §6 had
no consistency rule to back it - added one, closing the exact kind of
dangling-pointer gap this PR exists to prevent.
- Moved the products.js-vs-style-guide-table precedence into §13 once;
kb-pr-review's product-names row carried it inline while kb-pr-open's
didn't - now both rows are identical and inherit it from the rulebook.
- Nit: clarified the Error: prefix exemption (b) is actually a special
case of (a), not an independent condition - it's named separately
because it's the specific pattern the log-dump rule produces.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- Derek's output contract had no channel for the soft/judgment
findings it now inherits from §12's semantic title reframes
(product-name-in-title, article-type/title mismatch) - previously a
counted hard-check row, now correctly a non-auto-apply soft
reminder, but Derek's "every issue is a table row" contract gave it
nowhere to go. Added a fourth soft-reminder note type alongside the
three knowledge_article_id states.
- Clarified the naming collision between "Derek" as a findings-table
label (used by kb-pr-open/kb-pr-review for their own inline
links/images checks) and the /derek skill (which doesn't check
links/images at all as of this PR). Added a one-line naming note to
both PR skills.
- Verified §14's "confirmed against real KB files" claim for the
Resolution-first classification order — it holds: 3 real articles
(excluding the known multi-template file) have Overview alongside
Symptom/Cause/Resolution, so the rationale text is accurate and
wasn't changed. Documented the mirror-direction edge case (Q&A
article with a stray Overview heading) that does affect exactly the
one template file, so it isn't mistaken for a new bug later.
- CLAUDE.md's skills table still described /derek's pre-PR scope;
updated to include callouts and bolding/path formatting.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- derek/SKILL.md's closing line ("never respond with anything beyond
the assessment line and output table") forbade the four soft-
reminder note types the Output section just above it defines,
including the new semantic-title-reframe note. Under a strict
reading, product-name-in-title and article-type/title-mismatch could
vanish from Derek's output entirely. Amended to explicitly allow the
defined soft reminders.
- §12's title-case Vale-deferral read as an unconditional instruction,
but it only resolves the duplicate-finding problem for skills that
run Vale in the same report. A standalone /derek invocation doesn't
run Vale, so on an H1-bearing article a mis-cased title still goes
unchecked - not a regression (Derek's own Vale-handles list already
excluded heading case), but the new text implied a deferral that
doesn't exist for that invocation path. Made the condition explicit.
- Nit: clarified derek/SKILL.md's "seven-area scope" claim is a total
count, not a row-by-row correspondence with the Overview line's
list (which groups keyword/description under frontmatter and splits
article type/structure differently than the table's §14/§15 rows).
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- §12's title-case bullet inverted its own premise: it said Derek's
"what Vale already handles" list "excludes heading case," when the
list actually assigns heading case to Vale (that's why a
/derek-only run misses it). As written, an agent resolving the
contradiction literally would start flagging H1 title case in every
invocation, duplicating Vale. Fixed the wording to state the premise
correctly.
- The "Naming note" added to both PR skills undercounted the /derek
scope delta - it named only links/images, but formatting: lists
(§3+§10) and prose-directness (§4) are also Derek-table areas that
/derek itself doesn't check. Both skills now name all four.
- Restored WARN to the raw-log-dump title trigger's level-token list -
both deleted source copies had it, the consolidated §12 dropped it
incidentally during extraction.
- Added a parenthetical to both skills' "do NOT flag heading case"
line clarifying it means H1 case only - the §12 no-H1 carve-out
(check the frontmatter title directly) isn't covered by Vale and
still applies, so the two lines don't actually conflict.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- §13's product-ID carve-out header said "three values are exceptions
to the strict-match rule" but the first bullet (onesecure) said to
flag and correct it - contradicting both the header and its own
sibling bullets' "never flag." Restructured so only general/
recoveryad are labeled carve-outs; onesecure is explicitly "always
flag."
- Scan row 8's "repetition-signaled clusters" claim had no home left
in §1 Gate A after extraction - the base rule's thin "Remove X"-
cluster merge criterion never made it into the consolidated section.
Restored it as Gate A's second merge criterion.
- The precedence preamble claimed exactly two deliberate rulebook-vs-
style-guide elaborations; two more exist in the extracted text
(sidebar_label truncation as a mechanical Required fix vs. the style
guide's softer "may be shortened," and the knowledge_article_id
presence-is-informational policy vs. the style guide's unqualified
Required-field listing). Added both to the list.
- Synced kb-pr-review's scan-row label wording to match kb-pr-open's
exactly (both now say "as of this writing; recount if...").
- Moved the "don't flag > **NOTE:** blockquotes as :::note admonitions"
line out of kb-pr-review's title:semantic row (where it has nothing
to do with titles) into rulebook §5 Callout severity, its actual
home; kb-pr-open never had the stray copy.
- Added a one-line carve-out to the rulebook preamble noting Derek
doesn't apply §8/§16, so "apply during the derek review" doesn't
read as covering areas Derek's own Review Areas table excludes.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- Rulebook preamble's Derek-scope carve-out named only two excluded
sections (§8, §16) as if that were the complete exclusion list, when
Derek's Review Areas table actually includes just six sections total
(§5, §6, §12-§15). Restated as an inclusion list - the same fix
already applied to both PR skills' "Naming note" - so the preamble
isn't a third, less accurate copy of the same scope statement.
- The heading-case Vale-deferral parenthetical (added last round to
both PR skills) never reached derek/SKILL.md itself. Derek's own
"what Vale already handles" list still said heading case
unqualified, which would make an agent resolve the no-H1 exception
against its own closer, more specific instruction and skip the
finding §12 exists to catch. Added the same parenthetical to Derek's
list entry.
- Added an explicit "Derek reports; it never edits" guard. The
rulebook sections Derek now reads are written in fixer voice for the
applying skills ("convert/relabel," "must be wrapped in backticks")
with no per-skill framing to signal report-only intent anymore.
- Nits: added line-number guidance for Derek's two new areas (§5, §6);
consolidated the sidebar_label truncation rule to one copy in §13,
with §12 pointing to it instead of restating it.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
… one fix
§13 requires frontmatter title, H1, and sidebar_label to agree, but
nothing said to keep them in sync when correcting just one — including
a Vale HeadingCase fix that rewrites only the H1 line. That fix would
otherwise introduce a fresh §13 violation immediately after Derek's
pass reported clean, with nothing in the same run left to catch it.
Added an explicit "fix all three together" instruction to §12's
mechanical-fixes list; both kb-pr-open and kb-pr-review inherit it
automatically since they already route title:mechanical to §12.
The other item from this round (Derek's frontmatter/title scope
correctly growing louder on ~300 legacy hyphenated-product-ID files)
is a real, intended behavior change already documented as accepted
migration debt in the rulebook preamble - not a defect, no fix needed.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- Derek's output contract had no channel for §13's low-priority
keyword observation (a keyword absent from the body but a plausible
search term) - explicitly not a Required fix per §13's own text, but
also not one of Derek's two defined soft-reminder types. Added it as
a third soft-reminder bullet, matching the severity kb-pr-open
already handles correctly for the same rule.
- Nit: Derek's links/images exclusion rationale ("outside Derek's
single-file review model") didn't actually distinguish those checks
from two areas Derek does own - the §13 products.js lookup and the
Product Names "verify against product docs" check are also outside
the article file. Rewrote the rationale around what actually
differs: link/image resolution is an unbounded search across
docs/kb/**, while products.js and product-docs checks are one fixed
lookup against one known source.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- §14's Q&A+Overview edge-case note pointed at
docs/kb/accessanalyzer-2601/kb-article-template.md as the one file
that hits it - verified that file actually has Symptom/Cause/
Resolution headings too, so under the Resolution-first rule it
classifies as Resolution, not the Q&A/Overview collision the note
describes. Verified corpus-wide the edge case hits zero files
(every ## Question+## Overview file also has a Symptom/Cause/
Resolution heading). Rewrote the note to state that accurately
instead of citing a file that demonstrates the opposite.
- "Heading case ... H1 case only" (derek) and the matching parenthetical
in both PR skills understated NetwrixKB.HeadingCase's actual scope -
it's scope: heading, firing on every heading level, not just the H1.
As written, the do-not-re-flag list read as license to flag H2-H6
case as a new finding, duplicating Vale. The real gap is narrower:
an article with no body H1 at all, where Vale's heading-scoped rule
never runs on anything. Fixed all three copies to state the correct
scope and the correct (narrower) gap.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
…report
Both skills' Overview-table row-label instructions baked "as of this
writing; recount if the table changes" directly into the literal
string used as the row label - that maintenance note would render
verbatim in the posted PR review comment. Moved it to prose next to
the instruction, kept the row labels short (matching the example
output blocks, which already used the short form).
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- §16's exclusion from Derek was justified by an "unbounded filesystem
lookup" rationale that only actually applies to the location rule.
external-refs (is this URL an external CDN link) and alt-text (does
alt text just repeat the filename) are pure single-file text checks,
same cost class as everything else in Derek's scope - dropping them
had no real justification. Moved both into Derek's scope; kept
location with kb-pr-open/kb-pr-review since it genuinely requires
checking the filesystem beyond the article. Updated Derek's areas
table, Overview line, description frontmatter, line-number guidance,
and the rulebook preamble's inclusion list to match. This also fixed
the "four areas" nit in both PR skills' naming notes for free - the
excluded set is now exactly four rows (links, images: location,
formatting: lists, prose-directness), matching the stated count.
- The precedence preamble's "four deliberate deviations" list missed
two real ones: §6's action-target-only bolding rule (contradicts the
style guide's unconditional "bold for UI elements...") and §13's
products.js-authoritative rule (the style guide's own example
frontmatter uses a non-canonical underscored value). Added both so
a future reader doesn't "fix" either back to the literal reading.
- PR description now lists four /derek behavior changes instead of
two: the previously-undocumented product-name-in-title downgrade
(counted finding -> uncounted note) and the newly-in-scope callout-
indent/bolding-consistency rules, plus a note on the images scope
split.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- derek/SKILL.md's Derek-vs-Overview reconciliation note was stale
against last round's Overview rewrite: pointed "below" at a line
that's actually above it, and described the Overview as folding
keyword/description into frontmatter and not itemizing images -
neither true anymore, since the Overview now lists all eight areas
explicitly. Simplified to state the now-exact match (8 table+Product
Names areas = 8 Overview items) instead of reconciling against
wording that no longer exists.
- The "authoring notes must not appear in the report" cleanup two
rounds ago covered the scan-table and cross-section row labels but
missed the Derek (N checks) bullet's own "21 as of this writing;
recount if..." clause in the same list, in both PR skills - the
exact kind of partial fix this PR exists to close out. Stripped it
and extended the standalone authoring note to cover all three counts
(21/23/6) instead of just two.
- §16's "do not flag file format" guard sat inside the Location bullet
- the one row Derek is explicitly told to skip - while Derek now
reviews the other two rows (external-refs, alt-text), one of which
(alt-text) uses a .png example. Promoted the guard to a lead-in
covering all three rows regardless of which skill reads which row.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
- §12's title-case Vale deferral was keyed on which skill is running,
not on whether Vale actually produced output for this file. Both PR
skills explicitly continue when Vale is missing or errors (kb-pr-open:
"Vale | not run", kb-pr-review: "Vale | Not available - skipped"),
so on a machine without Vale the deferral silently dropped the check
entirely - a real regression versus base derek/kb-pr-open/kb-pr-review,
which all checked title case unconditionally with no deferral. Fixed
to key the deferral on Vale having actually run and reported, with
an explicit fallback to checking the H1 directly when it didn't.
Synced into both PR skills' parenthetical clarifications.
- derek/SKILL.md's "eight areas = eight Overview items" arithmetic
check was coincidental, not a real correspondence - the two lists
partition the same scope differently (Overview collapses article
type/structure into one item; the table splits it into two rows).
The match would silently stop meaning anything the next time either
list changes. Dropped the count assertion rather than keep an
invariant that isn't actually one.
- kb-pr-review lacked kb-pr-open's findings-routing rule (scan-table
and cross-section findings route into the Derek table with
kb-editing-conventions/cross-section as the area, not their own
section) - a pre-existing gap, cheap to close now that both tables
are otherwise synced.
- PR description now lists the §15/§13 checks (Q&A question-format,
How-To structure, wrong-shape, title/H1 exact-match) that grew
standalone /derek's scope beyond what was previously called out.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
§12's title-case bullet told /derek two incompatible things in the
same paragraph - a bold clause saying /derek should check H1 title
case directly (no deferral to make for a standalone invocation), and
a parenthetical immediately after saying nothing changes for /derek
because it already defers heading case to Vale. derek/SKILL.md:13
sided with the parenthetical. Since /derek never runs Vale, a strict
reading of the bold clause would have made every H1-bearing article
with bad title case a new Required-fix row - a mass finding class
introduced by my own fix two rounds ago, and not mentioned in the PR
description's behavior-change list because it wasn't intended.
Resolved by scoping the whole bullet to kb-pr-open/kb-pr-review (the
skills that actually run Vale) and removing /derek from its "no
deferral" clause entirely, leaving derek/SKILL.md:13's existing
Vale-handles-heading-case-except-no-H1 policy as the sole authority
for what /derek checks - no new mass-finding class, no contradiction.
Also: disambiguated "the canonical list in src/config/products.js" to
name the products array specifically, since the file also has a
categories array whose ids do use hyphens (directory-management,
endpoint-management, identity-management) - zero live impact today
(no KB file uses those values) but removes a live ambiguity.
Generated with AI
Co-Authored-By: Claude Code <ai@netwrix.com>
@github-actions

Copy link
Copy Markdown
Contributor

Code Review

No correctness issues found. This PR touches only agent instruction/reference markdown (.claude/references/kb-editing-conventions.md, three SKILL.md files, one CLAUDE.md table row) — no source, config, script, or workflow changes, so there is no build, routing, KB-pipeline, or secret-handling surface.

Details of what I checked, since a "pure refactor" of instruction files can still lose rules or leave dangling pointers:

No CI side effects. The four .claude/** files plus root CLAUDE.md fall outside every .md-triggered workflow's scope: vale-autofix.yml and claude-doc-pr.yml filter to docs/**/*.md, and md-extension-autofix.yml filters its changed-file list to ^docs/ before running the fixer. Nothing will try to auto-lint or auto-rewrite the rulebook. Only claude-code-review.yml fires (it uses paths-ignore on docs/), which is expected.

Cross-references all resolve. The rulebook defines §1–§16; the union of §N references across all four files is §1–§8 and §10–§16 — no out-of-range pointers. The refactor also cleaned up every reference to the section numbering it deleted: no remaining hits for derek/SKILL.md §N, "Title-format rules — two categories", or "knowledge_article_id rules in Step 4".

Extraction is lossless. Each deleted block has a landing site: Derek's old §1→§13, §2→§15, §3→§12, §5→§6, §6→§15 (Resolution Option Structure), §7→§5, §8→§13; Product Names stays as its own section. kb-pr-open/kb-pr-review's 15 areas-table cells all map to a rulebook section carrying the same substance, including the additions that used to live only in those cells (§3's "applies broadly" to Resolution sub-sections, §10's fenced-block rule, §4's impersonal-subject bullet).

Factual claims that gate behavior check out against the repo:

  • §8's backstop wording matches docusaurus.config.js:67-69onBrokenLinks: targetProduct ? 'warn' : 'throw', onBrokenMarkdownLinks: 'throw', onBrokenAnchors: 'throw'. Worth calling out that this fixes a pre-existing factual error: the old links rows in both PR skills claimed a single-product build "relaxes all three to warn," which was wrong for the latter two.
  • BoilerplateCrossRef.yml exists only under .vale/styles/Netwrix/, and .vale.ini's [docs/kb/**/*.md] section overrides BasedOnStyles to NetwrixKB — so §8's unreachability claim holds. WeakLinkText.yml is in both style dirs, so keeping the WeakLinkText search rule is correct.
  • NetwrixKB/HeadingCase.yml is scope: heading, as §12 and Derek's Vale-handles list state.
  • src/config/products.js: no product id contains a hyphen or underscore; 1secure exists and onesecure does not; category ids do use hyphens, including the exact directory-management example §13 cites.
  • scripts/copy-kb-to-versions.mjs:73-74 maps both recoveryforactivedirectory and identityrecovery to recoveryad, as §13's ambiguity carve-out claims.
  • kb_style_guide.md:211 is indeed - enterprise_auditor, matching the preamble's "known gap in the style guide's example."
  • Carve-out file counts are exact: onesecure 5, general 44, recoveryad 1. Non-NOTE/IMPORTANT blockquote severities: 16 files, matching §5's "~16."

Counts and arithmetic are self-consistent.Derek (N checks) = 21 (7 frontmatter sub-fields + 14 areas) matches the 15-row areas table; the scan table is 23 rows; cross-section is 6 patterns; Overview "fixed 5 rows" = Vale + Dale + Derek + scan + cross-section. §11 has exactly 5 bullets, so the new "all six rows map to §11's five bullets" note is accurate.

§14's load-bearing ordering is sound, and its documented edge case is real but harmless. I checked the corpus: exactly two files carry a Q&A heading alongside a Symptom/Cause/Resolution heading. One is the acknowledged multi-template file docs/kb/accessanalyzer-2601/kb-article-template.md; the other, docs/kb/dataclassification/troubleshooting-and-errors/export-not-available-for-dsar-searches.md, has ## Symptom + ## Cause + ## Answer and genuinely is a Resolution article using the wrong third heading. Resolution-first classifies it correctly and §15 flags the missing ## Resolution — so the ordering produces the right finding rather than a false positive. No file has ## Question followed by a Resolution-family heading.

Nit (non-blocking)

kb-editing-conventions.md §15 states 105 ## Causes; the current count is 102 (## Resolutions + ## Symptoms together are 154, against the stated 97 + 96). The aggregate is hedged as "~171," and this is a migration-debt statistic that doesn't change any rule's behavior, so it's cosmetic — but the per-heading numbers read as precise.

@hilram7
hilram7 merged commit 286a5a9 into devAug 12, 2026
7 checks passed
@hilram7
hilram7 deleted the kb-skills-dedup branch August 12, 2026 21:23
Sign up for freeto 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.

1 participant

@hilram7