Skip to content

refactor(platform): iterate GetDeepValue path traversal - #1647

Merged
skevetter merged 1 commit into
mainfrom
refactor/surface-parameter-path-traversal
Oct 22, 2026
Merged

skevetter merged 1 commit into
mainfrom
refactor/surface-parameter-path-traversal

Conversation

@skevetter

@skevetter skevetter commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Rewrites GetDeepValue in pkg/platform/parameters/parameters.go as one iterative loop over strings.SplitSeq(path, ".") with a type switch on map[string]any and []any.
  • Removes the private helpers getDeepValueFromMap and getDeepValueFromSlice. Their only caller was GetDeepValue.
  • Adds 5 table cases to TestGetDeepValue: empty-string key, empty-segment literal key, nil value mid-path, items.+1, items.01.

Why

  • The recursive version re-joined and re-split the path at each level. The loop removes both and keeps one traversal.
  • Behavior is unchanged. A throwaway differential test compared old and new over 52,200 root/path combinations, and all matched. The throwaway test was deleted.
  • Callers cmd/pro/workspace/import.go and pkg/platform/form/form.go are unchanged.

Validation

  • mise run cli:format: exit 0.
  • mise run cli:lint:ci: exit 0. Runs golangci-lint config verify and golangci-lint run --new-from-patch over ./..., so caller packages were type-checked by lint.
  • go test -count=1 ./pkg/platform/parameters/...: pass. pkg/platform/form and cmd/pro/workspace have no test files, so their own tests did not run.
  • prek run on the two changed files (pre-commit stage): all applicable hooks passed (golangci-lint, golangci-lint-fmt, end-of-file, trailing-whitespace, merge-conflict, large-files, secrets). prek commit-msg stage on the commit message: commitlint and commitizen passed.
  • Non-test diff: 57 changed lines (+19/-38).

CodeRabbit CLI

  • coderabbit review --agent --committed --base-commit c3952565: Review completed, 0 findings, 2 files reviewed.
  • Log on the worker host: /tmp/cr.log.

Auto-merge

  • Eligible under the auto-refactor policy. Pending coordinator gates: CI, ready, review gates, merge.

Next

  • None.

Summary by CodeRabbit

  • Bug Fixes
    • Deep-value lookups now handle paths through nested maps and slices. Invalid or out-of-range slice indices, missing keys, and unsupported values return nil.
    • Empty paths return the original value.

@skevetter skevetter added the surface-refactor Unattended refactor (surface) label Oct 22, 2026
@netlify

netlify Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit e4253cf
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6ad9b2737f2af334e1f25f2b

@netlify

netlify Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit e4253cf
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6ad9b2739ee67d07302e21cb

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 43489bd9-eb69-46b3-9249-fd6b428a01ff
📥 Commits

Reviewing files that changed from the base of the PR and between c395256 and e4253cf.

📒 Files selected for processing: 2
  • pkg/platform/parameters/parameters.go
  • pkg/platform/parameters/parameters_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

📝 Walkthrough

GetDeepValue now traverses nested maps and slices iteratively. Tests add coverage for empty paths, empty key segments, nil intermediate values, and slice index formats.

Changes: Deep Value Traversal

Layer / File(s) Summary
Iterative path traversal
pkg/platform/parameters/parameters.go, pkg/platform/parameters/parameters_test.go
GetDeepValue uses iterative map and slice traversal. Tests add cases for empty paths, empty key segments, nil intermediate values, and slice indices with a leading plus sign or zero padding.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to e4253

The traversal preserves the checked empty-path behavior, and no merge-blocking issue is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the iterative refactor of GetDeepValue path traversal.
Description check Passed The description clearly explains the change, rationale, scope, behavior verification, and validation results. It omits several template headings and checklist confirmations, but the substantive inform…
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.
Full details: Description check

Explanation

The description clearly explains the change, rationale, scope, behavior verification, and validation results. It omits several template headings and checklist confirmations, but the substantive information is mostly complete.

✨ Finishing Touches
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@skevetter
skevetter marked this pull request as ready for review October 22, 2026 08:48
@skevetter

Copy link
Copy Markdown
Contributor Author

@greptileai review

@mergify

mergify Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@greptile-apps

greptile-apps Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This PR appears safe to merge; no actionable issues were found.

Summary

GetDeepValue now walks each path in one loop instead of rebuilding the path for recursive calls.

  • Nested parameter lookups walk dotted paths one segment at a time.

Reviews (1) · Last reviewed commit: "refactor(platform): iterate GetDeepValue..." · Reviewed by Greptile

@skevetter

Copy link
Copy Markdown
Contributor Author

CodeRabbit docstring-coverage warning: no code change. GetDeepValue retains its existing exported API and repository lint passes; the removed helpers need no documentation. The auto-refactor style policy favors self-documenting code and does not require new docstrings here. CodeRabbit completed review of e4253cf with no actionable comments; Greptile reviewed the same head at 5/5. Local prek also passed on both changed files.

@skevetter
skevetter merged commit 7b37c31 into main Oct 22, 2026
115 checks passed
@skevetter
skevetter deleted the refactor/surface-parameter-path-traversal branch October 22, 2026 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m surface-refactor Unattended refactor (surface)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant