feat: implement okf standard#25
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds OKF as a second topic mode, with mode-aware scaffolding, promotion and conformance checks, relative link rendering, CLI commands, official bundle viewers, and QA review manifests. ChangesOKF Dual-Mode Feature
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/config/config_test.go (1)
40-64: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winWrap these new config cases in
t.Run("Should...")subtests.The OKF assertions add meaningful coverage, but they extend
_test.gocases without the required subtest structure, and the normalization scenario is a good fit for a small table-driven case. As per coding guidelines,**/*_test.go: "Default to table-driven tests with focused helpers andt.TempDir()for filesystem isolation." As per path instructions,**/*_test.go: "MUST uset.Run("Should...")pattern for ALL test cases."Also applies to: 109-183
🤖 Prompt for 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. In `@internal/config/config_test.go` around lines 40 - 64, Wrap the new config assertions in `TestDefaultConfigHasValidDefaults` and the related normalization coverage in `t.Run("Should...")` subtests so all `_test.go` cases follow the required pattern. Keep the existing checks in `Default()`/`Validate()`-based tests, but move each scenario into its own focused `t.Run` block, using table-driven subtests if helpful for the normalization case. Ensure the added OKF assertions remain covered inside a `Should...` subtest and preserve the current test intent while matching the testing convention.Sources: Coding guidelines, Path instructions
internal/generate/generate.go (1)
451-457: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDon’t hardcode wiki mode in generated topic metadata.
linkFornow dispatches throughLinkFormatterFor(topic), so forcingMode: wikihere makes downstream renders emit wiki-style links even when the resolved topic is OKF. Carry the actual topic mode intomodels.TopicMetadatainstead.🤖 Prompt for 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. In `@internal/generate/generate.go` around lines 451 - 457, The generated topic metadata is hardcoding the mode in the return value from the topic metadata builder, which causes downstream link rendering to always behave like wiki mode. Update the `models.TopicMetadata` construction in the generator so it carries the actual resolved topic mode instead of forcing `models.TopicModeWiki`, and make sure the mode value comes from the topic/source being processed so `LinkFormatterFor(topic)` can render correctly.internal/topic/topic.go (1)
552-558: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve
modewhen rewritingtopic.yaml.
WriteMetadataFilealways passes"", so the YAML omitsmode.readTopicMetadatathen falls back to wiki, which will silently downgrade an OKF topic the next time a caller rewrites metadata through this helper.Also applies to: 560-566
🤖 Prompt for 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. In `@internal/topic/topic.go` around lines 552 - 558, `WriteMetadataFile` is always dropping the topic mode by calling `topicMetadataForRef(..., "")`, which causes rewritten `topic.yaml` files to lose `mode`. Update `WriteMetadataFile` (and the related caller path in `topicMetadataForRef` / any helper it feeds) to preserve and pass through the existing mode instead of hardcoding an empty string, so rewrites keep OKF topics from falling back to wiki.
🧹 Nitpick comments (3)
internal/okf/okf_test.go (1)
15-214: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap test cases in
Should...subtests.These scenarios are direct top-level test bodies; the repo test instructions require
t.Run("Should...")for all test cases. The repeated strict/lenient check cases are also good candidates for a small table. As per path instructions,MUST use t.Run("Should...") pattern for ALL test cases; as per coding guidelines, default to table-driven tests.🤖 Prompt for 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. In `@internal/okf/okf_test.go` around lines 15 - 214, The test cases in TestPromoteWritesConceptIndexAndLog, TestPromoteRejectsNonOKFTargetBeforeWriting, TestPromoteWarnsWhenSourceBodyHasNoSentenceFallback, TestCheckReportsConformanceAndStrictWarnings, and TestCheckAllowsLenientExternalBundleTraits are written as top-level bodies instead of required Should... subtests. Wrap each scenario in t.Run with a Should... name, and for the repeated Check variants in TestCheckReportsConformanceAndStrictWarnings, prefer a small table-driven set of subtests to keep the strict and non-strict assertions together.Sources: Coding guidelines, Path instructions
internal/cli/topic_test.go (1)
82-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required subtest pattern for the new mode cases.
These additions are a good fit for a small table-driven block with
t.Run("Should..."), which is the test shape required in this repo.As per coding guidelines, "
**/*_test.go: Default to table-driven tests with focused helpers andt.TempDir()for filesystem isolation." As per path instructions, "**/*_test.go: MUST use t.Run("Should...") pattern for ALL test cases."🤖 Prompt for 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. In `@internal/cli/topic_test.go` around lines 82 - 140, The new topic mode tests in topic_test.go should be wrapped in the repo-required subtest structure instead of standalone test functions. Refactor TestTopicNewCommandPassesOKFMode and TestTopicNewCommandRejectsInvalidMode into a small table-driven block using t.Run with names that start with "Should...", keeping the same assertions and setup around newRootCommand, runTopicNewWithMode, and topicGetwd. Ensure each case remains isolated and continues to validate both the accepted okf mode and the invalid mode error path.Sources: Coding guidelines, Path instructions
internal/vault/pathutils_test.go (1)
199-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
t.Run("Should...")consistently in the new test cases.This block mixes ad-hoc assertions with subtests, and the subtest names do not follow the required
Should...pattern. Split the wiki/OKF/selector checks into named subtests so failures stay consistent with the repo’s test contract. As per path instructions,**/*_test.go“MUST use t.Run("Should...") pattern for ALL test cases.”🤖 Prompt for 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. In `@internal/vault/pathutils_test.go` around lines 199 - 235, Refactor TestLinkFormatters to wrap every assertion in t.Run subtests named with the required “Should...” prefix, instead of mixing direct assertions with ad-hoc checks. Keep the existing coverage for WikiLinkFormatter, OKFLinkFormatter, and LinkFormatterFor, but split them into clearly named subtests so failures are consistent with the test contract and easy to locate.Source: Path instructions
🤖 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 `@internal/cli/okf_integration_test.go`:
- Around line 80-87: The promotion log check in the test is too weak because it
can pass without verifying that InsertLogEntry actually recorded the promotion.
Update the assertion in okf_integration_test.go to look for a promotion-specific
outcome in log.md, using unique symbols like InsertLogEntry,
frontmatter.DateLayout, and the promoted topic/asset name, rather than the
literal year prefix or the generic **Creation** entry. Ensure the test fails
unless the promotion entry itself is present.
In `@internal/cli/okf_test.go`:
- Around line 48-75: The test for the promote command currently verifies flags
and output but does not confirm the positional source document is forwarded into
kokf.Promote. Update the assertion around the promote path in okf_test.go to
capture and verify the input passed to Promot(e) includes the source argument
from command args[0] (research/wiki/concepts/Alpha.md), so a regression in
positional-argument plumbing will fail the test.
In `@internal/cli/okf.go`:
- Around line 66-74: The okf check command currently ignores TopicInfo.Mode from
runOKFTopicInfo and always passes TopicInfo.RootPath into kokf.Check, so non-OKF
topics can be linted as directories instead of being rejected. Update the okf
check flow in the command handler to inspect the returned TopicInfo.Mode before
calling kokf.Check, fail fast for non-OKF topics, and only run the bundle check
when the topic is actually an OKF topic.
In `@internal/cli/topic_test.go`:
- Around line 127-140: The invalid-mode test in
TestTopicNewCommandRejectsInvalidMode is too loose because
strings.Contains(err.Error(), "invalid --mode") can match unrelated failures;
update the assertion to use the shared error-checking helper or compare the full
validation message from newRootCommand()/command.ExecuteContext so only the
intended --mode validation error passes. Keep the test anchored on the specific
invalid-mode path by asserting against the exact error content rather than a
substring.
In `@internal/okf/official_integration_test.go`:
- Around line 14-16: The official integration subtests are using raw bundle IDs
as names instead of the required behavior-oriented `Should...` pattern. Update
the `t.Run` call in `official_integration_test.go` so each case name follows the
`Should...` convention while still iterating over the same bundle list, keeping
the test logic in `t.Run` and the surrounding loop unchanged.
In `@internal/okf/okf_test.go`:
- Around line 216-224: The test helper assertIssue currently matches only
severity, file path, and target, so it can miss regressions in the actual
diagnostic text. Update assertIssue in okf_test.go to also accept an expected
message substring and verify it against each models.LintIssue’s
message/diagnostic text before returning. Then update the callers to pass the
expected message so the tests assert the exact behavior using the existing
assertIssue helper.
In `@internal/okf/okf.go`:
- Around line 94-128: Make concept path allocation atomic in
Promote/allocateConceptPath: the current os.Stat check can race with the later
os.WriteFile in Promot e, letting two concurrent promotions pick the same
concept path. Update allocateConceptPath to reserve the file with exclusive
creation (O_CREATE|O_EXCL) or otherwise atomically claim the path before
writing, and on collision retry the path selection and dependent transform using
conceptKey, allocateConceptPath, and os.WriteFile.
- Around line 577-580: In loadConcepts, the frontmatter.Parse error is currently
being swallowed by returning nil, which lets malformed concept files disappear
from the regenerated index. Update the logic around frontmatter.Parse so it
preserves and returns parse errors, and only skips files when there is truly no
frontmatter (for example, an empty values result without an error). Make sure
the handling in loadConcepts still supports RegenerateIndex but does not
silently drop concepts from malformed files.
In `@internal/topic/topic_test.go`:
- Around line 236-312: Wrap the new test coverage in named subtests using the
required t.Run("Should...") pattern in TestNewWithModeCreatesOKFTopicSkeleton
and TestReadTopicMetadataDefaultsMissingModeToWiki. Keep the existing assertions
and helpers, but move each case into a subtest block so the *_test.go file
follows the repo’s test conventions and remains table-driven/focused where
appropriate.
In `@internal/topic/topic.go`:
- Around line 437-440: The createTopicSkeleton helper is incorrectly
short-circuiting for TopicModeOKF, which skips the standard topic scaffold.
Update createTopicSkeleton so it always creates the normal topic directories and
base files owned by kb topic new, then apply any OKF-specific additions on top;
keep the scaffold generation for raw, wiki, outputs, bases, CLAUDE.md,
AGENTS.md, and log.md, and ensure raw/codebase remains present for later
inspection commands.
In `@internal/vault/render_test.go`:
- Around line 266-290: The OKF link assertions in
TestRenderDocumentsUseOKFMarkdownLinkSyntax are currently expecting root-scoped
paths instead of the true relative targets. Update the test to verify the actual
nested relative links produced by vault.RenderDocuments for the
raw/codebase/files/commands/run.ts.md document and the
wiki/codebase/index/Codebase Dashboard.md document, using the symbol and concept
link outputs from the fixture rather than the broken root-relative paths. Keep
the checks focused on the rendered document bodies and ensure they assert the
correct relative markdown destinations.
In `@internal/vault/render_wiki.go`:
- Around line 108-117: The OKF link generation in the wiki renderer is using an
empty fromDir, so links in rendered markdown are rooted at the topic instead of
relative to the current document. Update the linkFor calls in the wiki rendering
flow to pass the rendered document’s own directory as fromDir before building
links, especially for the codebase index and concept pages. Use the current
document context from the render function that emits wiki pages so
dashboard/index-to-concept and concept-to-concept links resolve with correct
relative paths. Apply the same fromDir fix to every linkFor invocation in this
render path, including the navigation and article link sections.
In `@internal/vault/render.go`:
- Around line 190-191: Update the OKF link generation path so `linkFor` receives
the current document directory instead of an empty root path. Thread the source
directory through `linkForNode` and any related callers that currently pass
fromDir as "", so `LinkFormatterFor(...).Link(...)` can build correct relative
links for nested raw documents. Focus on the `linkFor` and `linkForNode` flow in
`render.go` and preserve existing behavior outside OKF mode.
In `@internal/vault/writer.go`:
- Around line 475-477: The OKF bridge links in writer.go are being resolved from
the bundle root because linkFor is called with an empty source directory, which
breaks navigation for files rendered under wiki/index/. Update the three linkFor
calls in this block to pass wiki/index as the source directory so
GetWikiIndexPath(CodebaseDashboardTitle),
GetWikiIndexPath(CodebaseConceptIndexTitle), and
GetWikiIndexPath(CodebaseSourceIndexTitle) resolve correctly in OKF bundles.
---
Outside diff comments:
In `@internal/config/config_test.go`:
- Around line 40-64: Wrap the new config assertions in
`TestDefaultConfigHasValidDefaults` and the related normalization coverage in
`t.Run("Should...")` subtests so all `_test.go` cases follow the required
pattern. Keep the existing checks in `Default()`/`Validate()`-based tests, but
move each scenario into its own focused `t.Run` block, using table-driven
subtests if helpful for the normalization case. Ensure the added OKF assertions
remain covered inside a `Should...` subtest and preserve the current test intent
while matching the testing convention.
In `@internal/generate/generate.go`:
- Around line 451-457: The generated topic metadata is hardcoding the mode in
the return value from the topic metadata builder, which causes downstream link
rendering to always behave like wiki mode. Update the `models.TopicMetadata`
construction in the generator so it carries the actual resolved topic mode
instead of forcing `models.TopicModeWiki`, and make sure the mode value comes
from the topic/source being processed so `LinkFormatterFor(topic)` can render
correctly.
In `@internal/topic/topic.go`:
- Around line 552-558: `WriteMetadataFile` is always dropping the topic mode by
calling `topicMetadataForRef(..., "")`, which causes rewritten `topic.yaml`
files to lose `mode`. Update `WriteMetadataFile` (and the related caller path in
`topicMetadataForRef` / any helper it feeds) to preserve and pass through the
existing mode instead of hardcoding an empty string, so rewrites keep OKF topics
from falling back to wiki.
---
Nitpick comments:
In `@internal/cli/topic_test.go`:
- Around line 82-140: The new topic mode tests in topic_test.go should be
wrapped in the repo-required subtest structure instead of standalone test
functions. Refactor TestTopicNewCommandPassesOKFMode and
TestTopicNewCommandRejectsInvalidMode into a small table-driven block using
t.Run with names that start with "Should...", keeping the same assertions and
setup around newRootCommand, runTopicNewWithMode, and topicGetwd. Ensure each
case remains isolated and continues to validate both the accepted okf mode and
the invalid mode error path.
In `@internal/okf/okf_test.go`:
- Around line 15-214: The test cases in TestPromoteWritesConceptIndexAndLog,
TestPromoteRejectsNonOKFTargetBeforeWriting,
TestPromoteWarnsWhenSourceBodyHasNoSentenceFallback,
TestCheckReportsConformanceAndStrictWarnings, and
TestCheckAllowsLenientExternalBundleTraits are written as top-level bodies
instead of required Should... subtests. Wrap each scenario in t.Run with a
Should... name, and for the repeated Check variants in
TestCheckReportsConformanceAndStrictWarnings, prefer a small table-driven set of
subtests to keep the strict and non-strict assertions together.
In `@internal/vault/pathutils_test.go`:
- Around line 199-235: Refactor TestLinkFormatters to wrap every assertion in
t.Run subtests named with the required “Should...” prefix, instead of mixing
direct assertions with ad-hoc checks. Keep the existing coverage for
WikiLinkFormatter, OKFLinkFormatter, and LinkFormatterFor, but split them into
clearly named subtests so failures are consistent with the test contract and
easy to locate.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48271340-f5ae-4363-9d87-d8261e5ed6a2
⛔ Files ignored due to path filters (144)
.agents/skills/compozy/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/compozy/references/cli-reference.mdis excluded by!**/*.md,!.agents/**.agents/skills/compozy/references/config-reference.mdis excluded by!**/*.md,!.agents/**.agents/skills/compozy/references/skills-reference.mdis excluded by!**/*.md,!.agents/**.agents/skills/compozy/references/workflow-guide.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-create-prd/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-create-prd/references/question-protocol.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-create-tasks/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-create-tasks/references/task-context-schema.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-create-tasks/references/task-template.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-create-techspec/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-execute-task/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-execute-task/references/tracking-checklist.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-fix-reviews/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-idea-factory/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-idea-factory/references/adr-template.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-idea-factory/references/business-analyst.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-idea-factory/references/council.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-idea-factory/references/idea-template.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-idea-factory/references/product-strategist.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-idea-factory/references/question-protocol.mdis excluded by!**/*.md,!.agents/**.agents/skills/cy-review-round/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/git-rebase/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/git-rebase/references/automation.mdis excluded by!**/*.md,!.agents/**.agents/skills/git-rebase/references/resolution-patterns.mdis excluded by!**/*.md,!.agents/**.agents/skills/git-rebase/references/scripts-tools.mdis excluded by!**/*.md,!.agents/**.agents/skills/git-rebase/references/strategies.mdis excluded by!**/*.md,!.agents/**.agents/skills/git-rebase/references/troubleshooting.mdis excluded by!**/*.md,!.agents/**.agents/skills/git-rebase/scripts/analyze-conflicts.shis excluded by!.agents/**.agents/skills/git-rebase/scripts/pre-rebase-backup.shis excluded by!.agents/**.agents/skills/git-rebase/scripts/validate-merge.shis excluded by!.agents/**.claude/ledger/2026-06-27-MEMORY-okf-skill-sync.mdis excluded by!**/*.md,!.claude/**.compozy/tasks/okf-dual-mode/_prd.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/_techspec.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/adrs/adr-001.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/adrs/adr-002.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/adrs/adr-003.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/adrs/adr-004.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/adrs/adr-005.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/adrs/adr-006.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/impl-review-findings-round1.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/impl-review-findings-round2.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/impl-review-prompt-round1.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/impl-review-prompt-round2.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/impl-review-remediation-round1.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/impl-review-summary-round1.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/impl-review-summary-round2.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/peer-review-findings-round1.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/peer-review-findings-round2.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/peer-review-incorporation-round1.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/peer-review-incorporation-round2.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/peer-review-prompt-round1.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/peer-review-prompt-round2.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/peer-review-summary-round1.mdis excluded by!**/*.md.compozy/tasks/okf-dual-mode/qa/peer-review-summary-round2.mdis excluded by!**/*.mdAGENTS.mdis excluded by!**/*.mdCLAUDE.mdis excluded by!**/*.mdREADME.mdis excluded by!**/*.mdconfig.example.tomlis excluded by!**/*.tomlinternal/okf/testdata/official/LICENSE.mdis excluded by!**/*.mdinternal/okf/testdata/official/README.mdis excluded by!**/*.mdinternal/okf/testdata/official/crypto_bitcoin/datasets/crypto_bitcoin.mdis excluded by!**/*.mdinternal/okf/testdata/official/crypto_bitcoin/datasets/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/crypto_bitcoin/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/crypto_bitcoin/tables/blocks.mdis excluded by!**/*.mdinternal/okf/testdata/official/crypto_bitcoin/tables/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/crypto_bitcoin/tables/inputs.mdis excluded by!**/*.mdinternal/okf/testdata/official/crypto_bitcoin/tables/outputs.mdis excluded by!**/*.mdinternal/okf/testdata/official/crypto_bitcoin/tables/transactions.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/datasets/ga4_obfuscated_sample_ecommerce.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/datasets/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/joins/events___ads_clickstats.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/joins/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/avg_pageviews.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/avg_spend_per_purchase_session_by_user.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/avg_transactions_per_purchaser.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/day_count.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/event_count.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/new_user_count.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/overall_avg_spend_per_purchase_session.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/references/metrics/user_count.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/tables/events_.mdis excluded by!**/*.mdinternal/okf/testdata/official/ga4/tables/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/datasets/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/datasets/stackoverflow.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/badge_classes.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/close_as_off_topic_reason_types.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/close_reason_types.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/creative_commons_by_sa_4_0_license.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/flag_types.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/link_types.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/pending_flags.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/post_feedback.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/post_history_type_ids.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/post_notice_class_ids.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/post_notice_duration_ids.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/post_notice_type_definitions.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/post_notice_types.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/post_notices.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/post_type_ids.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/related_questions.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/review_rejection_reasons.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/review_task_result_types.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/review_task_results.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/review_task_states.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/review_task_types.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/review_tasks.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/sede_databases.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/sede_databases_markdown.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/sede_ineachdb.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/sede_sites.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/sede_tables.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/sede_users.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/suggested_edit_votes.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/suggested_edits.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/tag_synonyms.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/references/vote_types.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/badges.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/comments.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/index.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/post_history.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/post_links.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/posts_answers.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/posts_moderator_nomination.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/posts_orphaned_tag_wiki.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/posts_privilege_wiki.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/posts_questions.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/posts_tag_wiki.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/posts_tag_wiki_excerpt.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/posts_wiki_placeholder.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/stackoverflow_posts.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/tags.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/users.mdis excluded by!**/*.mdinternal/okf/testdata/official/stackoverflow/tables/votes.mdis excluded by!**/*.mdinternal/topic/assets/okf-claude-template.mdis excluded by!**/*.mdskills/kb/SKILL.mdis excluded by!**/*.mdskills/kb/references/architecture.mdis excluded by!**/*.mdskills/kb/references/frontmatter-schemas.mdis excluded by!**/*.mdskills/kb/references/okf-mode.mdis excluded by!**/*.md
📒 Files selected for processing (48)
.compozy/tasks/okf-dual-mode/qa/impl-review-changed-files-round1.txt.compozy/tasks/okf-dual-mode/qa/impl-review-changed-files-round2.txt.compozy/tasks/okf-dual-mode/qa/impl-review-diff-round1.patch.compozy/tasks/okf-dual-mode/qa/impl-review-diff-round2.patch.compozy/tasks/okf-dual-mode/qa/impl-review-diff-stat-round1.txt.compozy/tasks/okf-dual-mode/qa/impl-review-diff-stat-round2.txt.compozy/tasks/okf-dual-mode/qa/impl-review-events-round1.jsonl.compozy/tasks/okf-dual-mode/qa/impl-review-events-round2.jsonl.compozy/tasks/okf-dual-mode/qa/impl-review-result-round1.err.compozy/tasks/okf-dual-mode/qa/impl-review-result-round2.err.compozy/tasks/okf-dual-mode/qa/impl-review-status-after-round1.txt.compozy/tasks/okf-dual-mode/qa/impl-review-status-after-round2.txt.compozy/tasks/okf-dual-mode/qa/impl-review-status-before-round1.txt.compozy/tasks/okf-dual-mode/qa/impl-review-status-before-round2.txt.compozy/tasks/okf-dual-mode/qa/peer-review-events-round1.jsonl.compozy/tasks/okf-dual-mode/qa/peer-review-events-round2.jsonl.compozy/tasks/okf-dual-mode/qa/peer-review-result-round1.err.compozy/tasks/okf-dual-mode/qa/peer-review-result-round2.err.compozy/tasks/okf-dual-mode/qa/peer-review-status-after-round1.txt.compozy/tasks/okf-dual-mode/qa/peer-review-status-after-round2.txt.compozy/tasks/okf-dual-mode/qa/peer-review-status-before-round1.txt.compozy/tasks/okf-dual-mode/qa/peer-review-status-before-round2.txtinternal/cli/okf.gointernal/cli/okf_integration_test.gointernal/cli/okf_test.gointernal/cli/promote.gointernal/cli/root.gointernal/cli/topic.gointernal/cli/topic_test.gointernal/config/config.gointernal/config/config_test.gointernal/generate/generate.gointernal/models/kb_models.gointernal/models/models.gointernal/okf/official_integration_test.gointernal/okf/okf.gointernal/okf/okf_test.gointernal/okf/testdata/official/crypto_bitcoin/viz.htmlinternal/okf/testdata/official/ga4/viz.htmlinternal/okf/testdata/official/stackoverflow/viz.htmlinternal/topic/topic.gointernal/topic/topic_test.gointernal/vault/pathutils.gointernal/vault/pathutils_test.gointernal/vault/render.gointernal/vault/render_test.gointernal/vault/render_wiki.gointernal/vault/writer.go
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/cli/okf_test.go (1)
18-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required
t.Run("Should...")wrapper for these cases.Both tests assert real CLI behavior, but the top-level cases still bypass the mandated subtest pattern. As per path instructions,
**/*_test.go:MUST use t.Run("Should...") pattern for ALL test cases.Also applies to: 131-168
🤖 Prompt for 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. In `@internal/cli/okf_test.go` around lines 18 - 79, Wrap the CLI test cases in the required t.Run("Should...") subtest pattern instead of keeping them as top-level tests; update the relevant test functions in okf_test.go so each real behavior assertion runs inside a descriptive Should-prefixed subtest. Keep the existing setup and assertions, but move the command execution and result checks under the t.Run wrapper to satisfy the test convention for all *_test.go cases.Source: Path instructions
internal/vault/writer_test.go (1)
213-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap this case in
t.Run("Should...").This test hits the right behavior, but it skips the repo’s required subtest shape for Go tests. As per path instructions,
**/*_test.go:MUST use t.Run("Should...") pattern for ALL test cases.🤖 Prompt for 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. In `@internal/vault/writer_test.go` around lines 213 - 246, Wrap the body of TestWriteVaultUsesRelativeOKFTopicIndexBridgeLinks in a t.Run subtest whose name starts with "Should..." to match the repo-wide Go test pattern. Keep the existing assertions and setup inside the subtest, and use the same test function and helper symbols (TestWriteVaultUsesRelativeOKFTopicIndexBridgeLinks, testWriteVaultInputs, vault.WriteVault, readFile) so the case is still easy to locate.Source: Path instructions
🤖 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.
Nitpick comments:
In `@internal/cli/okf_test.go`:
- Around line 18-79: Wrap the CLI test cases in the required t.Run("Should...")
subtest pattern instead of keeping them as top-level tests; update the relevant
test functions in okf_test.go so each real behavior assertion runs inside a
descriptive Should-prefixed subtest. Keep the existing setup and assertions, but
move the command execution and result checks under the t.Run wrapper to satisfy
the test convention for all *_test.go cases.
In `@internal/vault/writer_test.go`:
- Around line 213-246: Wrap the body of
TestWriteVaultUsesRelativeOKFTopicIndexBridgeLinks in a t.Run subtest whose name
starts with "Should..." to match the repo-wide Go test pattern. Keep the
existing assertions and setup inside the subtest, and use the same test function
and helper symbols (TestWriteVaultUsesRelativeOKFTopicIndexBridgeLinks,
testWriteVaultInputs, vault.WriteVault, readFile) so the case is still easy to
locate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87afdde8-17d0-429f-9242-e3ff75d9a74c
⛔ Files ignored due to path filters (20)
.compozy/tasks/pr-25/reviews-001/issue_001.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_002.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_003.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_004.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_005.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_006.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_007.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_008.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_009.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_010.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_011.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_012.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_013.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_014.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_015.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_016.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_017.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_018.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_019.mdis excluded by!**/*.md.compozy/tasks/pr-25/reviews-001/issue_020.mdis excluded by!**/*.md
📒 Files selected for processing (18)
internal/cli/okf.gointernal/cli/okf_integration_test.gointernal/cli/okf_test.gointernal/cli/topic_test.gointernal/config/config_test.gointernal/generate/generate.gointernal/generate/generate_test.gointernal/okf/official_integration_test.gointernal/okf/okf.gointernal/okf/okf_test.gointernal/topic/topic.gointernal/topic/topic_test.gointernal/vault/pathutils_test.gointernal/vault/render.gointernal/vault/render_test.gointernal/vault/render_wiki.gointernal/vault/writer.gointernal/vault/writer_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- internal/okf/official_integration_test.go
- internal/vault/pathutils_test.go
- internal/config/config_test.go
- internal/cli/okf.go
- internal/vault/render_test.go
- internal/cli/okf_integration_test.go
- internal/cli/topic_test.go
- internal/vault/writer.go
- internal/okf/okf.go
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 by CodeRabbit
okf checkCLI command to validate OKF bundles withtable/json/tsvoutput and--strictbehavior.topic newwith--mode wiki|okf, plus addedokf.typesconfiguration for allowed OKF concept types.