docs: admin data browser proposed -> implemented (Phase 6)#817
Conversation
Lifecycle transition for docs/design/2026_05_22_admin_data_browser.md. All six phases of the rollout plan shipped between 2026-05-22 and 2026-05-24: Phase 2a — DynamoDB item adapter RPCs (#805) Phase 2b — S3 object adapter RPCs (#811) Phase 3a — DynamoDB item HTTP + bridge (#813) Phase 3b — S3 object HTTP + bridge (#814) Phase 4 — SPA DynamoDetail Items tab (#815) Phase 5 — SPA S3Detail Objects tab + Upload (#816) Phase 6 — this rename (#817) Doc updates: - Status: Proposed -> Implemented - Implementation record table at the top with one row per PR + the notable in-flight design adjustments that came out of PR review (empty L/M MarshalJSON, percent-encoded URL segments, URL-key schema-awareness limit, dist gitignore). - Rollout plan table now carries an Outcome column with the PR number for each phase. Per CLAUDE.md convention: git mv preserves history; the file moves from _proposed_ to _implemented_ rather than being deleted and re-added.
Adds the content updates the rename commit (82d9387) was supposed to include but missed because the content edits happened after the git add of the rename: - Status: Proposed -> Implemented - Date stamp: 2026-05-22 (proposed) -> 2026-05-24 (implemented) - Implementation record table with PR numbers per phase - Notable in-flight design adjustments from PR review - Rollout plan Outcome column with PR numbers
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 42 minutes and 32 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA design document for the admin data browser feature is updated to reflect implementation status, with a new section recording which backend and SPA phases shipped across dated PRs and documenting implementation details, alongside a revised rollout plan table with explicit outcomes. ChangesAdmin Data Browser Design Implementation Update
Estimated Code Review Effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the design document for the Admin Web UI for DynamoDB and S3 to reflect its "Implemented" status. It adds a detailed implementation record mapping project phases to specific pull requests and notes design adjustments made during the process. A review comment suggests correcting a typo in the description of DynamoDB HTTP handlers for better clarity and requests further detail on operational risks associated with the implementation bridge.
| |-------|-------------|----| | ||
| | 2a | DynamoDB item adapter RPCs (`AdminScanTable` / `AdminGetItem` / `AdminPutItem` / `AdminDeleteItem`) | #805 | | ||
| | 2b | S3 object adapter RPCs (`AdminListObjects` / `AdminGetObject` / `AdminPutObject` / `AdminDeleteObject`) | #811 | | ||
| | 3a | DynamoDB item HTTP handlers + bridge (custom `MarshalJSON` preserves empty `L`/`M`; NUL-byte and trailing-data PUT-body guards; URL-key percent-encoding accepted, decoded-`/` / dot-segments still rejected; leader-churn translation matches the table-side bridge) | #813 | |
There was a problem hiding this comment.
The phrasing "decoded-/ / dot-segments" contains a typo. It is clearer to explicitly name the characters being rejected, as shown in the suggestion. Furthermore, as this design specifies a "bridge" for DynamoDB handlers, ensure the document clearly details the operational risks (like rolling upgrades) this bridge is intended to mitigate, as per the repository's standards for design documentation.
| | 3a | DynamoDB item HTTP handlers + bridge (custom `MarshalJSON` preserves empty `L`/`M`; NUL-byte and trailing-data PUT-body guards; URL-key percent-encoding accepted, decoded-`/` / dot-segments still rejected; leader-churn translation matches the table-side bridge) | #813 | | |
| | 3a | DynamoDB item HTTP handlers + bridge (custom MarshalJSON preserves empty L/M; NUL-byte and trailing-data PUT-body guards; URL-key percent-encoding accepted, decoded "/" and dot-segments still rejected; leader-churn translation matches the table-side bridge) | #813 | |
References
- When a design document identifies a significant operational risk, such as the inability to perform rolling upgrades, it must also detail potential mitigation strategies, like implementing a temporary "bridge" or "proxy" mode.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@docs/design/2026_05_22_implemented_admin_data_browser.md`:
- Around line 1-5: The document filename doesn't match its implemented date;
rename the file docs/design/2026_05_22_implemented_admin_data_browser.md to
docs/design/2026_05_24_implemented_admin_data_browser.md so the filename aligns
with the implemented date shown in the file; update any references if present to
the old filename (search for 2026_05_22_implemented_admin_data_browser or the
document title "Admin Web UI for DynamoDB Item and S3 Object CRUD") to point to
the new filename to restore naming/traceability consistency.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c53965d-a3ee-42e1-b590-7e0ba9d0da2d
📒 Files selected for processing (1)
docs/design/2026_05_22_implemented_admin_data_browser.md
| # Admin Web UI for DynamoDB Item and S3 Object CRUD | ||
|
|
||
| **Status:** Proposed | ||
| **Status:** Implemented | ||
| **Author:** bootjp | ||
| **Date:** 2026-05-22 | ||
| **Date:** 2026-05-22 (proposed) → 2026-05-24 (implemented) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the filename of the admin data browser design doc
fd -t f 'admin_data_browser\.md$' docs/design/Repository: bootjp/elastickv
Length of output: 117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Matching implemented admin data browser docs:"
fd -t f '2026_05_.._implemented_admin_data_browser\.md$' docs/design/ || true
echo
echo "Matching proposed admin data browser docs:"
fd -t f '2026_05_.._proposed_admin_data_browser\.md$' docs/design/ || true
echo
echo "Check for 2026-05-24 references inside the reviewed file:"
rg -n "2026-05-24|2026_05_24" docs/design/2026_05_22_implemented_admin_data_browser.md || true
echo
echo "Search repo for the specific filename mentioned in the review comment (05_24 target):"
rg -n "2026_05_24_implemented_admin_data_browser\.md" -S docs/design .github . || trueRepository: bootjp/elastickv
Length of output: 475
Rename implemented doc to match the 2026-05-24 implemented date
docs/design/2026_05_22_implemented_admin_data_browser.md exists, but the file’s implemented date is 2026-05-24 and there are no files/references for docs/design/2026_05_24_implemented_admin_data_browser.md. Rename the file to docs/design/2026_05_24_implemented_admin_data_browser.md to restore traceability with the repo’s naming convention/PR objective.
🤖 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 `@docs/design/2026_05_22_implemented_admin_data_browser.md` around lines 1 - 5,
The document filename doesn't match its implemented date; rename the file
docs/design/2026_05_22_implemented_admin_data_browser.md to
docs/design/2026_05_24_implemented_admin_data_browser.md so the filename aligns
with the implemented date shown in the file; update any references if present to
the old filename (search for 2026_05_22_implemented_admin_data_browser or the
document title "Admin Web UI for DynamoDB Item and S3 Object CRUD") to point to
the new filename to restore naming/traceability consistency.
r1) Gemini medium on PR #817 - the original phrasing "decoded-` / ` / dot-segments" reads ambiguously (the inline-code slash next to the literal slash blurs the boundary). Rewrites the same fact as "decoded \`/\` and dot-segments still rejected" which mirrors the wording the path-validation tests use. CodeRabbit's separate suggestion to rename the file to 2026_05_24_implemented_admin_data_browser.md is intentionally NOT applied: per docs/design/README.md, the YYYY_MM_DD prefix is the **propose date** (the first commit that added the file). It stays at 2026-05-22 across the proposed/partial/implemented lifecycle so git mv preserves history and the directory listing keeps the canonical propose-date ordering. The implemented date lives inside the file (`Date:` header) instead of in the filename. Replied to the bot comment with the same explanation.
|
@claude review Round 1 addresses Gemini medium (typo on the path-validation phrasing in the implementation record). CodeRabbit's separate suggestion to rename the file from |
|
Claude finished @bootjp's task in 1m 38s —— View job Code Review — PR #817 (Phase 6: proposed → implemented)
Gemini r1 fix (path-validation phrasing) — correctly applied ✓The Phase 3a implementation table row (line 17) now reads:
The bullet-point narrative (line 25) uses the encoded forms for precision:
Both formulations are accurate and unambiguous. The r1 commit ( CodeRabbit filename suggestion — correctly rejected ✓The suggestion to rename from
The Minor:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c63ad70d6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **Status:** Implemented | ||
| **Author:** bootjp | ||
| **Date:** 2026-05-22 | ||
| **Date:** 2026-05-22 (proposed) → 2026-05-24 (implemented) |
There was a problem hiding this comment.
The header date now includes two values (2026-05-22 ... 2026-05-24) instead of a single YYYY-MM-DD, which conflicts with the repository convention in docs/design/README.md that the header Date must match the filename’s propose date. This makes metadata inconsistent for humans and any tooling that parses design-doc headers expecting one canonical date.
Useful? React with 👍 / 👎.
…817 r2) Codex P2 on PR #817. The r1 header carried two values ("2026-05-22 (proposed) -> 2026-05-24 (implemented)") which conflicted with the docs/design/README.md convention that the header Date matches the filename propose date — one canonical YYYY-MM-DD value, with the rest of the lifecycle living inside the document body. Fix: Date: line reverts to 2026-05-22 (matches the filename propose date prefix). The implemented date moves into the Implementation record paragraph, where the per-phase PR table already carries enough context to date the milestones.
|
@claude review Round 2 addresses Codex P2 (header Date format). Reverted the header |
|
Claude finished @bootjp's task in 1m 8s —— View job Code Review — PR #817 Round 2 (Codex P2: Date header fix)
Codex P2 fix (Date header) — correctly applied ✓Line 5 now reads: This is the single propose-date form, matching the filename prefix and satisfying
The implemented-on date is correctly relocated to the Implementation record paragraph (lines 11–12):
The information is preserved where it's contextually richer (alongside the per-phase PR table), and the header no longer conflicts with the single-date convention. r1 Gemini phrasing fix — still intact ✓
No regression from the Round 1 commit. CodeRabbit filename rename suggestion — still correctly rejected ✓
No issues found in Round 2. The doc is correct and consistent with the established conventions. |
…I flake (#818) ## Summary Fixes the recurring `TestRedis_ExpiredKey_BecomesInvisible` CI flake that has been blocking unrelated admin PRs (#813, #814, #815, #816) all day. ## Root cause The test sets a key with `PX 200` (200ms TTL) then immediately reads it back: ```go require.NoError(t, rdb.Do(ctx, "SET", "expiry:short", "v", "PX", "200").Err()) got, err := rdb.Get(ctx, "expiry:short").Result() require.NoError(t, err) // ← fails: redis.Nil require.Equal(t, "v", got, "key must be visible before expiry") ``` The `SET` goes through a 3-node Raft cluster: client → leader proposes → quorum apply → leader applies → response. Under `-race` on CI runners this round-trip can take 100–250ms. The TTL clock starts at the moment the leader applies, so by the time `SET` returns OK on a slow CI runner, the key may have <50ms of TTL left. The immediately-following `GET` then loses the race and hits `redis.Nil`. That this is the bug — not anything in any of #813-#816 — is locked in by the cross-PR failure pattern: the flake fired today on PRs touching only `internal/admin/` and `web/admin/`, with zero code overlap with `adapter/`. ## Fix `PX 200 → PX 2000`. 2s is comfortably past the worst observed SET-ack latency (~250ms on CI) while keeping the test under a 7s wall-clock budget (2s TTL + 5s `Eventually` deadline, raised from 1s). The test's intent — verify expired keys become invisible to subsequent GETs — is preserved exactly. Only the timing constants change; the assertions are unchanged. ## Self-review (5 passes) 1. **Data loss** — none. Test-only constants. 2. **Concurrency** — closes a wall-clock vs. Raft-apply-latency race in the test fixture. 3. **Performance** — test now takes ~3s instead of ~250ms locally; still well under the 30s package timeout. 4. **Consistency** — the broader test still asserts both halves (visible before expiry, invisible after expiry) of the TTL contract. 5. **Test coverage** — no change to coverage; same assertions, friendlier timing. ## Test plan - [x] `go test -race -count=3 -timeout=180s -run TestRedis_ExpiredKey_BecomesInvisible ./adapter/` — passes 3/3 (10.792s) - [x] `golangci-lint --config=.golangci.yaml run` — clean - [ ] CI ## Context Discovered while running the PR review loop on #813-#817. The Claude bot's Round 8 review on #813 marked the PR as "Ready to merge once CI passes" — only the Redis flake was blocking. This fix unblocks #813 (and the parallel admin PRs) without touching any admin code. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved reliability of the Redis TTL compatibility test by adjusting timing parameters to reduce intermittent test failures. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/bootjp/elastickv/pull/818?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
](https://app.coderabbit.ai/change-stack/bootjp/elastickv/pull/818?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
1 participant
Summary
Phase 6 — close the loop on the Admin Data Browser rollout:
git mv docs/design/2026_05_22_proposed_admin_data_browser.md → _implemented_…Proposed→ImplementedSequencing
This PR should merge last, after Phases 2a (#805 merged), 2b (#811 merged), 3a (#813), 3b (#814), 4 (#815), 5 (#816) have all landed. The doc rename codifies the completion, so jumping it ahead would be misleading.
Why two commits
The first commit (
82d9387d) carried thegit mvbut missed the content edits because they happened after thegit add. The second commit (bb7b77be) adds the content. They're kept separate per CLAUDE.md's "never amend, prefer new commits" rule — both are tracked in the same PR.What changed in the doc
Implementation record table:
Notable in-flight design adjustments documented:
L/MDynamo attributes preserved via customMarshalJSON(Gemini medium, Codex P1 on internal/admin: HTTP handlers for DynamoDB item RPCs (Phase 3a) #813)url.PathUnescapebefore validating; URL-encoded table names work, decoded//..still rejected (Codex P1 on internal/admin: HTTP handlers for DynamoDB item RPCs (Phase 3a) #813).gitignored; CI rebuilds from sourceTest plan
git mvpreserves history (rename detection 100% similarity on the first commit)go test -count=1 ./...— passes (no code changes)Phase plan (final state)
Summary by CodeRabbit