Skip to content

docs: improve COMMENT diagram readability#29

Open
ebembi-crdb wants to merge 16 commits into
masterfrom
demo/diagram-replace-example
Open

docs: improve COMMENT diagram readability#29
ebembi-crdb wants to merge 16 commits into
masterfrom
demo/diagram-replace-example

Conversation

@ebembi-crdb
Copy link
Copy Markdown
Owner

Demo: SQL Diagram Automation

This PR demonstrates a typical writer workflow - improving diagram readability.

The Change

In the COMMENT statement diagram, replace the internal grammar term 'SCONST' with the user-friendly label comment_text.

Before: The diagram shows SCONST (string constant - confusing for users)
After: The diagram shows comment_text (clear and descriptive)

What This Demonstrates

  1. Single file edit - Only diagrams.go changed (no Bazel files!)
  2. Automatic CI - Workflow triggers on diagrams.go changes
  3. PNG previews - See the updated diagram right in this PR
  4. Validation - Broken links, naming conflicts caught before merge
  5. Auto-sync - Bot creates PR in generated-diagrams repo

This is a demo PR showcasing the SQL diagram automation workflow.

ebembi-crdb and others added 16 commits December 10, 2025 14:41
This commit introduces an automated pipeline for SQL diagram generation
that simplifies the workflow for technical writers.

Changes:
- Add CI workflow (.github/workflows/docgen-diagrams-ci.yml) that:
  - Triggers on changes to diagrams.go, sql.y, or the docgen-diagram label
  - Generates BNF files from sql.y
  - Builds SVG diagrams via Railroad Diagram Generator
  - Runs comprehensive validation tests
  - Packages and uploads artifacts
  - Triggers bot to sync diagrams to generated-diagrams repo
  - Comments on PRs with validation status

- Add validation test suite (pkg/cmd/docgen/tests/):
  - TestMissingBNFFiles: Detects missing BNF files
  - TestMissingHTMLDiagrams: Detects missing HTML output
  - TestStmtNamingConflicts: Detects _stmt naming conflicts
  - TestReplaceUnlinkMismatch: Warns about potential broken links
  - TestBrokenGrammarLinks: Finds invalid grammar references
  - TestUnusedDiagrams: Reports diagrams not used in docs
  - TestSKIPDOCSuppressions: Reports SKIP DOC annotations

- Add bot sync script (build/scripts/sync-diagrams-to-generated-repo.sh):
  - Clones generated-diagrams repository
  - Updates BNF and SVG files
  - Creates PR with validation summary (no auto-merge)
  - Assigns PR to docs-infra-prs team

- Add writer documentation (docs/generated/sql/bnf/README.md):
  - Instructions for adding/modifying diagrams
  - Explains what writers don't need to do (no Bazel commands)
  - Troubleshooting guide

- Add technical documentation (docs/tech-notes/sql-diagram-automation.md):
  - Architecture overview with diagram
  - CI workflow details
  - Bot service specification
  - Secret configuration
  - Troubleshooting and rollback procedures

New writer workflow:
1. Edit pkg/cmd/docgen/diagrams.go only
2. Open PR with label docgen-diagram
3. Add remote_include manually only for new diagrams

Release Notes: None

Epic: None
…t repo

This commit improves the SQL diagram automation CI workflow:

1. Added changed_files output to track which diagrams changed
2. Updated manifest.json to include changed files as a JSON array
3. Made target repository configurable via DIAGRAM_TARGET_REPO variable
   (defaults to cockroachdb/generated-diagrams)
4. Enhanced PR comment to show list of changed diagrams
5. Updated TestUnusedDiagrams stub to clarify it's a placeholder for
   future functionality

Epic: None
Release note: None
This commit removes the pull_request_target event trigger from the
SQL diagram generation workflow to address a security vulnerability.

The issue was that pull_request_target grants access to secrets and
runs with write permissions. When combined with checking out untrusted
PR code and executing scripts from it (./dev, ./build/github/*.sh),
this created a vector for secret exfiltration and runner compromise.

Changes:
- Remove pull_request_target trigger (used for docgen-diagram label)
- Remove label check from should_run logic
- Add comment explaining the security rationale
- workflow_dispatch remains available for manual triggering by maintainers

The pull_request trigger (for path changes) remains safe since it does
not have access to secrets.

Epic: None
Release note: None
docgen: Implement complete SQL diagram automation pipeline
This test change adds 'opt_collate' to the inline list for the add_column
diagram to demonstrate the SQL diagram automation workflow.

Epic: None
Release note: None
This commit is FORK-SPECIFIC and should NOT be upstreamed.

Changes for running on forks without CockroachDB infrastructure:
- Use ubuntu-latest instead of self-hosted runners
- Add Bazel installation
- Skip EngFlow keys (internal to CockroachDB)

Epic: None
Release note: None
Testing the automated SQL diagram generation workflow with GitHub-hosted runners.

Epic: None
Release note: None
- Add Java 11 setup (required for Railroad JAR)
- Download and configure Railroad Diagram Generator JAR
- Set COCKROACH_REQUIRE_RAILROAD environment variable
- Add fallback build command with explicit JAR location

This should enable HTML/SVG diagram generation on GitHub-hosted runners.

Epic: None
Release note: None
The unzip was conflicting with existing LICENSE file in the repo.
Now extracting in a temporary directory to avoid conflicts.

Epic: None
Release note: None
Adding diagnostic output to understand why HTML files aren't being generated.

Epic: None
Release note: None
Testing the workflow with debug output to see why HTML files aren't generated.

Epic: None
Release note: None
The dev wrapper requires initialization which doesn't work well on
GitHub-hosted runners. Using bazel directly for:
- BNF generation
- SVG diagram building
- Validation tests

This should fix the HTML generation issue.

Epic: None
Release note: None
Replace internal grammar term 'SCONST' with user-friendly label
'comment_text' in the COMMENT statement diagram.

This makes the diagram easier to understand for users reading
the SQL documentation.

Release note: None
Epic: None
@github-actions
Copy link
Copy Markdown

SQL Diagram Generation Report

Validation Passed

🔄 Diagram changes detected
A PR has been opened to sync these changes: cockroachdb/generated-diagrams#26

SKIP DOC Warnings (click to expand)

The following grammar rules are suppressed from documentation:

The following grammar rules are suppressed with SKIP DOC:
1977:| alter_virtual_cluster_stmt   /* SKIP DOC */
2570:    /* SKIP DOC */
2582:    /* SKIP DOC */
2707:    /* SKIP DOC */
2724:    /* SKIP DOC */
3035:    /* SKIP DOC */
3041:    /* SKIP DOC */
3093:    /* SKIP DOC */
3205:  SET CYCLE                        { /* SKIP DOC */
3591:    /* SKIP DOC */
3612:  INCLUDE_ALL_SECONDARY_TENANTS { /* SKIP DOC */ }
4197:  TENANT  { /* SKIP DOC */ }
4201:  TENANT_NAME { /* SKIP DOC */ }
4792:    /* SKIP DOC */
4935:  /* SKIP DOC */
4974:    /* SKIP DOC */
4981:    /* SKIP DOC */
4989:    /* SKIP DOC */
4999:    /* SKIP DOC */
5011:  TENANT { /* SKIP DOC */ }
6203:    /* SKIP DOC */
6241:    /* SKIP DOC */
6318:    /* SKIP DOC */
6441:    /* SKIP DOC */
6727:   /* SKIP DOC */
6736:    /* SKIP DOC */
7158:    /* SKIP DOC */
7169:    /* SKIP DOC */
7174:    /* SKIP DOC */
7179:    /* SKIP DOC */
7198:    /* SKIP DOC */
7203:    /* SKIP DOC */
7210:    /* SKIP DOC */
7215:    /* SKIP DOC */
7220:    /* SKIP DOC */
7225:    /* SKIP DOC */
7232:    /* SKIP DOC */
7239:    /* SKIP DOC */
7254:		/* SKIP DOC */
7264:    /* SKIP DOC */
7269:    /* SKIP DOC */
7276:    /* SKIP DOC */
7295:    /* SKIP DOC */
7328:    /* SKIP DOC */
7786:| set_exprs_internal   { /* SKIP DOC */ }
8091:    /* SKIP DOC */
8120:    /* SKIP DOC */
8221:    /* SKIP DOC */
8230:    /* SKIP DOC */
8239:    /* SKIP DOC */
8299:    /* SKIP DOC */
8306:    /* SKIP DOC */
8316:    /* SKIP DOC */
8321:    /* SKIP DOC */
8346:    /* SKIP DOC */
8352:    /* SKIP DOC */
8429:    /* SKIP DOC */
8437:    /* SKIP DOC */
8442:		/* SKIP DOC */
8451:    /* SKIP DOC */
8472:    /* SKIP DOC */
8477:    /* SKIP DOC */
8885:    /* SKIP DOC */
8890:    /* SKIP DOC */
8895:    /* SKIP DOC */
8900:    /* SKIP DOC */
8905:    /* SKIP DOC */
8936:| TRACING { /* SKIP DOC */ }
8939:    /* SKIP DOC */
8981:    /* SKIP DOC */
9002:    /* SKIP DOC */
9054:    /* SKIP DOC */
9060:    /* SKIP DOC */
9066:		/* SKIP DOC */
9072:		/* SKIP DOC */
9089:    /* SKIP DOC */
9094:    /* SKIP DOC */
9099:    /* SKIP DOC */
9177: /* SKIP DOC */
9239:    /* SKIP DOC */
9244:    /* SKIP DOC */
9743:    /* SKIP DOC */
9908:    /* SKIP DOC */
9916:    /* SKIP DOC */
9929:    /* SKIP DOC */
9939:   /* SKIP DOC */
9972:    /* SKIP DOC */
9977:    /* SKIP DOC */
9982:    /* SKIP DOC */
10022:    /* SKIP DOC */
10031:    /* SKIP DOC */
10040:    /* SKIP DOC */
10045:    /* SKIP DOC */
10050:    /* SKIP DOC */
10055:    /* SKIP DOC */
10060:    /* SKIP DOC */
10069:    /* SKIP DOC */
10079:    /* SKIP DOC */
10255:| for_with_lookahead_variants { /* SKIP DOC */ }
10262:| FOR_JOB { /* SKIP DOC */ }
10457:    /* SKIP DOC */
10466:    /* SKIP DOC */
10775:    /* SKIP DOC */
10784:    /* SKIP DOC */
11093:    /* SKIP DOC */
11109:    /* SKIP DOC */
11199:| WITH DATA    { /* SKIP DOC */ /* This is the default */ }
11590:    /* SKIP DOC */
12097:| CYCLE                        { /* SKIP DOC */
12153:    /* SKIP DOC */
12234:    /* SKIP DOC */
12257:    /* SKIP DOC */
12535:    /* SKIP DOC */
12541:    /* SKIP DOC */
12546:    /* SKIP DOC */
12551:    /* SKIP DOC */
13575:| abort_stmt               /* SKIP DOC */
13905:    /* SKIP DOC */
13920:    /* SKIP DOC */
14066:    /* SKIP DOC */
14567:    /* SKIP DOC */
14990:    /* SKIP DOC */
14995:    /* SKIP DOC */
15000:    /* SKIP DOC */
15022:    /* SKIP DOC */
15027:    /* SKIP DOC */
15040:    /* SKIP DOC */
15045:    /* SKIP DOC */
15120:    /* SKIP DOC */
15206:    /* SKIP DOC */
15535:    /* SKIP DOC */
15607:    /* SKIP DOC */
15636:    /* SKIP DOC */

This comment was generated by the SQL Diagram CI workflow.

Sign up for free to 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