Repository navigation
fix(docs): review fixes for Authentication pilot (framework, nav, structure) - #15
Merged
Merged
Conversation
…erflow Three fixes from review of the Authentication pilot and the new architectural-control doc type: - The mental-model blockquote skipped straight to "how platforms implement auth" without ever stating what authentication actually is. Rewritten to open with the three-factor framework (something you know / have / are, NIST SP 800-63-4) before anything else. Locked this as a requirement for the doc type going forward, not just a one-off pilot fix: ADR-0003 and the add-architectural-control skill now require every doc's blockquote to name its topic's real first-principles framework (RBAC/ABAC/ReBAC for authorization, the CIA triad and Kerckhoffs's principle for encryption, etc.) before covering the fork and the reviewer's core check. - A "diagram: omitted, <reason>" sentence, meant as process metadata reported back to the orchestrating conversation, had leaked into the shipped doc body as literal prose. Removed, and the skill's instructions now say explicitly that this call is response text, not document content. - Long, TLDR-repeating paragraphs in "Where this decision forks" and the context sections tightened: cut one paragraph that only restated points the strengthened blockquote and adjacent paragraphs already made, split run-ons, fixed comma splices. The skill now requires short (2-4 sentence) paragraphs here and prohibits restating the blockquote in different words, matching the conciseness rule already in place for the other doc type. Also: the top nav tab bar has 11 top-level sections now and overflows its container at essentially every desktop width with a bare overflow-x scrollbar and no visual cue, so the later tabs (Misconfiguration, Infrastructure, AI & LLM Security) were effectively undiscoverable. Added a CSS mask-image fade on both edges of .md-tabs__list so an overflowing bar visibly signals there's more to scroll to. Confirmed the overflow (scrollWidth 1426 vs clientWidth 1216, consistent from ~1220px up) and the fade rendering locally before shipping.
…eness rule Continues review of the Authentication pilot: - The prose in "Where this decision forks" and each of the four context sections had accumulated 3-4 back-to-back short paragraphs each making one parallel, independent point. No single paragraph was long, but the run of them read as a wall. Converted each such run into a short bulleted list with a bold lead-in per point, kept every worked compare-contrast example (fintech vs admin console, banking app vs habit tracker, service mesh vs marketing platform) as prose, since those read better as flowing narrative than fragmented bullets. - Removed a meta-commentary sentence opening "## Interviewer probes" that described the section's own format instead of starting on the first question. - The skill was missing the general CONCISENESS principle that add-appsec-topic has as its own named block; it only had scattered doc-specific rules. Added the same principle here, explicit that it applies to prose only (blockquote and context-opening paragraphs), never to table cells, which are already dense by design. Skill and post-run checklist updated to catch all of this on future architectural-control docs: convert parallel-paragraph runs to bulleted lists, no section-format meta-commentary, no prose paragraph past 5-6 sentences.
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
1 participant
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
Follow-up review fixes on top of the merged Authentication pilot (#14), on the same branch since #14 was merged before these landed:
Framework grounding (ADR-0003 + skill now require this for every future doc):
Prose structure:
CONCISENESS:principle that its siblingadd-appsec-topicskill has as a named block (only had scattered doc-specific rules). Added it, explicit that it applies to prose only, never to table cells.Nav bar:
overflow-xscrollbar and no visual cue, so later tabs (Misconfiguration, Infrastructure, AI & LLM Security) were effectively undiscoverable. Added a CSS mask-image fade on both edges of.md-tabs__list, verified locally against a screenshot before shipping.All of the skill/ADR changes here are locked in as requirements for every future architectural-control doc, not one-off fixes to this pilot.
Test plan
mkdocs build --strict— cleanpytest tests/— 11/11 passingMid:/Principal:probe format confirmed intact (8/8 pairs), unchanged by this PR🤖 Generated with Claude Code