fix(skill): Add no-serialized store framework in TAGS.md and skill by dashrews78 · Pull Request #21395 · stackrox/stackrox · GitHub
Skip to content

fix(skill): Add no-serialized store framework in TAGS.md and skill#21395

Open
dashrews78 wants to merge 3 commits into
masterfrom
dashrews/update-proto-tag-skill
Open

fix(skill): Add no-serialized store framework in TAGS.md and skill#21395
dashrews78 wants to merge 3 commits into
masterfrom
dashrews/update-proto-tag-skill

Conversation

@dashrews78

@dashrews78 dashrews78 commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Updates both reference files to cover the --no-serialized flag, sql:"strategy(bytea)"
tag, NoSerializedStore[T] interface, and WithChildren/WithoutChildren fetch options
introduced by the ROX-34569 serialized-free store generation work.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

review should be sufficient.

@dashrews78 @claude
docs(be): Document no-serialized store framework in TAGS.md and proto…
bb0867a 
…-writer skill

Updates both reference files to cover the --no-serialized flag, sql:"strategy(bytea)"
tag, NoSerializedStore[T] interface, and WithChildren/WithoutChildren fetch options
introduced by the ROX-34569 serialized-free store generation work.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dashrews78

dashrews78 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

@openshift-ci

openshift-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai

coderabbitai Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Documentation in proto/TAGS.md and .claude/skills/proto-writer/SKILL.md is extended to cover the --no-serialized store mode. Changes include tag constraint rules, a store-selection decision section, generator flag reference, and a complete scenario example with generated SQL and child fetch control.

Changes

No-Serialized Store Documentation

Layer / File(s) Summary
Tag rules and generator flag for --no-serialized
.claude/skills/proto-writer/SKILL.md, proto/TAGS.md		 Documents sql:"-" incompatibility, sql:"strategy(bytea)" for repeated message fields stored as a bytea blob in the parent table, the --no-serialized generator flag table entry, and adds proto and //go:generate examples illustrating usage.
Store selection and field-tag decision tree
.claude/skills/proto-writer/SKILL.md		 Updates the tag decision tree with a repeated-message strategy step, introduces a "Serialized vs No-Serialized Store" decision section with tradeoffs and an interactive prompt, and adds a no-serialized gen.go invocation example.
Full no-serialized scenario, type mapping, and code references
proto/TAGS.md		 Adds TOC entry and overview blurb, then adds the complete "No-Serialized Store" scenario section covering constraints, a full proto and gen.go example, generated SQL for parent and child tables, repeated-message strategies, WithoutChildren() child fetch control, a repeated Message → bytea type-mapping row, and code references to NoSerializedStore and messagebytes.go.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main documentation update for the no-serialized store framework.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description follows the template and clearly explains the doc-only changes, with all required sections present and a reasonable validation note.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dashrews/update-proto-tag-skill

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot reviewed Jun 24, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/skills/proto-writer/SKILL.md:
- Around line 31-38: The no-serialized guidance in the proto-writer SKILL.md
points readers to the wrong architectural step. Update the reference in the
“Creating a new no-serialized store” section so it points to Step 3c, not Step
3d, and make sure the wording stays aligned with the Step 3c architectural
decision used by the `NoSerializedStore[T]` guidance.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 4c3d3391-1e04-40d3-adf3-45e285947dc9

📥 Commits

Reviewing files that changed from the base of the PR and between 877de2a and bb0867a.

📒 Files selected for processing (2)
  • .claude/skills/proto-writer/SKILL.md
  • proto/TAGS.md

Comment thread .claude/skills/proto-writer/SKILL.md
@github-actions

github-actions Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🚀 Build Images Ready

Images are ready for commit caf7d69. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.12.x-301-gcaf7d69322

dashrews78 and others added 2 commits June 24, 2026 11:29
@dashrews78 @claude
docs(be): Trim no-serialized TAGS.md section to match existing verbosity
22b0c81 
Removed bullet lists, strategies table, and Go caller examples that were
heavier than surrounding scenario sections. Simplified proto example.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dashrews78 @claude
docs(be): Fix review findings in TAGS.md and proto-writer skill
caf7d69 
- Fix wrong cross-reference: Step 3d -> Step 3c
- Remove premature recommendation for no-serialized as default
- Fix orphaned type mapping table row (blank line separator)

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dashrews78 dashrews78 marked this pull request as ready for review June 24, 2026 15:49
@dashrews78

dashrews78 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci

openshift-ci Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown

@dashrews78: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/gke-qa-e2e-tests caf7d69 link false /test gke-qa-e2e-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@charmik-redhat charmik-redhat charmik-redhat approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dashrews78 @charmik-redhat