Skip to content

refactor(featurebundle): simplify merge surface via MergeBundles/Append helpers#120

Merged
matdev83 merged 1 commit into
mainfrom
pr4/featurebundle-merge-surface-simplify
Jul 8, 2026
Merged

refactor(featurebundle): simplify merge surface via MergeBundles/Append helpers#120
matdev83 merged 1 commit into
mainfrom
pr4/featurebundle-merge-surface-simplify

Conversation

@matdev83

@matdev83 matdev83 commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Reduces duplication in the feature-bundle merge surface so every new FeatureBundle field requires exactly one append line instead of three (length budget + Grow + append + struct field assignment).

What changed

  • internal/featurebundle/merge_surface.go:
    • Add MergedFeatureSurface.Append(b) — the single merge point for all slice fields.
    • Add MergeBundles(bundles...) helper that folds bundles in order.
    • Extract buildEnabledFeatureBundles (registry walk) from MergeFeatureSurface; MergeFeatureSurface now delegates to MergeBundles.
    • Drop the manual slices.Grow length-budgeting (premature optimization; append growth is sufficient and far less duplicated).
  • internal/featurebundle/merge_surface_test.go (new): cover empty merge, all-field concatenation, and bundle-order preservation across slices.
  • docs/feature-bridge-retirement-checklist.md: note the MergeBundles/Append simplification (plus an accuracy fix for the already-merged standardbundle façade deletion).

Verification (PR scoped, in isolation)

  • go test ./internal/featurebundle/... — passes (3 new tests: TestMergeBundles_empty, TestMergedFeatureSurfaceAppend_concatenatesAllFields, TestMergeBundles_preservesBundleOrderAcrossSlices).
  • go vet ./internal/featurebundle/... — clean.
  • go build ./... — passes; consumers (runtimebundle, standardplugins) build and test cleanly.

No behavior change; merge order and field contents are preserved.

Fourth of the PRs splitting the post-review cleanup changes on the local working tree.

…nd helpers

Reduce duplication in the feature-bundle merge surface so every new
FeatureBundle field requires exactly one append line instead of three
(length budget + Grow + append + struct field assignment).

- internal/featurebundle/merge_surface.go:
  - Add MergedFeatureSurface.Append(b) — the single merge point for all
    slice fields.
  - Add MergeBundles(bundles...) helper that folds bundles in order.
  - Extract buildEnabledFeatureBundles (registry walk) from MergeFeatureSurface;
    MergeFeatureSurface now delegates to MergeBundles.
  - Drop the manual slices.Grow length-budgeting (premature optimization;
    append growth is sufficient and far less duplicated).
- internal/featurebundle/merge_surface_test.go (new): cover empty merge,
  all-field concatenation, and bundle-order preservation across slices.
- docs/feature-bridge-retirement-checklist.md: note the MergeBundles/Append
  simplification (and the already-merged standardbundle façade deletion).

No behavior change; merge order and field contents are preserved.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7d4522af-5244-46e8-9274-aeff64830343

📥 Commits

Reviewing files that changed from the base of the PR and between 69941c1 and 2a78ea2.

📒 Files selected for processing (3)
  • docs/feature-bridge-retirement-checklist.md
  • internal/featurebundle/merge_surface.go
  • internal/featurebundle/merge_surface_test.go
📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: qa
🧰 Additional context used
📓 Path-based instructions (4)
**/*

📄 CodeRabbit inference engine (Custom checks)

Do not introduce hardcoded credentials, API keys, tokens, private keys, passwords, production secrets, or sensitive internal URLs.

Files:

  • docs/feature-bridge-retirement-checklist.md
  • internal/featurebundle/merge_surface_test.go
  • internal/featurebundle/merge_surface.go
**/*.go

📄 CodeRabbit inference engine (Custom checks)

**/*.go: For server, CLI, worker, or network Go code, ensure context.Context is propagated correctly, cancellation is respected, and new goroutines cannot leak indefinitely.
Do not make accidental public API breaks in Go code: under pkg/** or anywhere exported Go identifiers are changed, warn if the PR changes exported types, function signatures, error behavior, JSON fields, CLI flags, config keys, or documented behavior without clearly explaining the compatibility impact.

Files:

  • internal/featurebundle/merge_surface_test.go
  • internal/featurebundle/merge_surface.go

⚙️ CodeRabbit configuration file

**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.

Files:

  • internal/featurebundle/merge_surface_test.go
  • internal/featurebundle/merge_surface.go
internal/**

⚙️ CodeRabbit configuration file

internal/**: Focus on package boundaries, hidden coupling, unexported API design, concurrency safety, deterministic behavior, and whether logic belongs in this internal package.

Files:

  • internal/featurebundle/merge_surface_test.go
  • internal/featurebundle/merge_surface.go
**/*_test.go

⚙️ CodeRabbit configuration file

**/*_test.go: Review tests for meaningful assertions, table-driven coverage, race-prone tests, t.Parallel misuse, nondeterminism, leaked goroutines, real network or filesystem dependencies, fragile sleeps, and missing edge cases. Prefer testing observable behavior over implementation details.

Files:

  • internal/featurebundle/merge_surface_test.go
🧠 Learnings (1)
📚 Learning: 2026-07-01T22:57:42.953Z
Learnt from: matdev83
Repo: matdev83/go-llm-interactive-proxy PR: 101
File: pkg/lipsdk/scope/context.go:0-0
Timestamp: 2026-07-01T22:57:42.953Z
Learning: In this repository, when defining unexported Go `context` key constants of type `ctxKey int` (e.g., `const ( kFoo ctxKey = iota + N )`), preserve the `iota + <offset>` pattern and keep the existing `<offset>` values rather than simplifying to plain `iota`. These per-package offsets are part of the repo-wide convention to avoid key collisions across packages, and each such constant set should include a short explanatory comment (for example: “offset avoids collision with other packages' context keys”).

Applied to files:

  • internal/featurebundle/merge_surface_test.go
  • internal/featurebundle/merge_surface.go
🔇 Additional comments (3)
docs/feature-bridge-retirement-checklist.md (1)

32-32: LGTM!

internal/featurebundle/merge_surface.go (1)

44-106: LGTM!

internal/featurebundle/merge_surface_test.go (1)

1-257: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for combining multiple feature bundles into one merged surface.
    • Improved bundle handling so feature hooks and observers are appended in order.
  • Bug Fixes

    • Updated startup flow to use the current standard plugin installation path.
    • Removed an outdated bridge component from the retirement checklist.
  • Tests

    • Added coverage for empty merges, bundle concatenation, and order preservation.

Walkthrough

Refactors internal/featurebundle/merge_surface.go to introduce MergedFeatureSurface.Append and MergeBundles for incremental bundle merging, delegating MergeFeatureSurface to a new buildEnabledFeatureBundles helper. Adds unit tests for the new merge API and updates the bridge retirement checklist documentation accordingly.

Changes

Feature bundle merge refactor

Layer / File(s) Summary
Append and MergeBundles implementation
internal/featurebundle/merge_surface.go
Adds Append method and MergeBundles function for incremental slice-field concatenation; refactors MergeFeatureSurface to use new buildEnabledFeatureBundles helper, removing prior slices.Grow-based preallocation logic.
Unit tests for merge behavior
internal/featurebundle/merge_surface_test.go
Adds no-op test doubles and three tests validating empty merges, field concatenation across bundles, and preserved bundle order.
Retirement checklist documentation update
docs/feature-bridge-retirement-checklist.md
Updates the “Bridge status” line to reflect the merge surface simplification, façade package deletion, and direct standardplugins.InstallStandardBundleOn call.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the featurebundle merge-surface refactor and the new MergeBundles/Append helpers.
Description check ✅ Passed The description clearly matches the refactor, tests, and docs updates in the changeset.
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.
No Secrets ✅ Passed Modified files contain only merge/refactor code and docs text; no hardcoded secrets, tokens, passwords, private keys, or sensitive URLs found.
Context Propagation ✅ Passed The PR only refactors synchronous slice concatenation; no new goroutines, and existing bootstrap ctx still flows into tracing/shutdown paths.
No Accidental Public Api Break ✅ Passed Only new helpers were added in an internal package; no existing exported signatures or pkg/** fields changed, and the PR states behavior is preserved.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@matdev83 matdev83 merged commit f14779d into main Jul 8, 2026
2 checks passed
matdev83 pushed a commit that referenced this pull request Jul 8, 2026
…rdrails, EchoesVault

Documentation closure for the go-llm-interactive-proxy architecture
de-slopification effort. Updates the spec docs and project knowledge
base to reflect the end state of the refactor.

- specs/go_llm_proxy_arch_review/resolution-plan.md: mark all phases
  complete; rewrite the plan as the final closure record.
- specs/go_llm_proxy_arch_review/{architecture-review,findings-register,
  source-index}.md: update to reflect deleted BuildExecutor wrappers,
  moved standard_table.go, and the feature merge surface.
- docs/adr/0001-registry-driven-composition.md: registry is now the
  registry type only; standard tables live in standardplugins; add a
  dated update entry for the standardbundle/types.go/BuildExecutor
  cleanup.
- docs/architecture-guardrails.md: document the bootstrap_plan.go
  composition-root exemption, the role metadata in the hexagonal
  baseline, and the standardplugins.DefaultWireModel rename.
- docs/architecture.md: note the MergeBundles/Append merge-surface
  simplification.
- internal/plugins/features/README.md: wiring location corrected from
  internal/pluginreg to internal/standardplugins.
- EchoesVault/pages/{plugin-system,testing-strategy}.md and
  EchoesVault/daily/2026-07-08.md: compiled knowledge base updated.

Docs only; no code or test changes.

Note: this PR describes the end state of the cleanup, which is delivered
across #118 (runtimebundle invariant boundary), #119 (baseline role),
and #120 (merge surface). It should merge after those three to keep
docs and code consistent.

Co-authored-by: Cursor <cursoragent@cursor.com>
@matdev83 matdev83 deleted the pr4/featurebundle-merge-surface-simplify branch July 13, 2026 14:10
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