Feat/cleanup admin#7
Merged
Merged
Conversation
Categories + techniques deduplication & restructuring via Gemini with a review-then-apply workflow. v1 explicitly excludes assets to avoid enrichment pipeline collisions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
17 tasks across backend (Go package, HTTP handlers, wiring) and frontend (Pinia store, three admin views, router). TDD for pure logic — prompt builder, parser, validator, reference-rewrite planners. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Required for ListJobs (disciplineId+entityType+createdAt desc, plus a 4-field variant when status filter is applied) and ListTemplates (disciplineId+entityType). Without these the queries fail in production with FAILED_PRECONDITION; the emulator is permissive and didn't catch it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Validator now drops any update where after.parentId or after.categoryIds[] references an id being deleted in the same job. Without this the executor would write a dangling reference in Pass 3 after the target record was already deleted in Pass 2. Adds two new test cases. All 10 TestValidate subtests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related fixes in applyUpdate: 1. C3 — re-check parent cycles against live Firestore state before writing parentId. Pass 2 may have moved records (e.g., reparenting children of a deleted category) in ways that create a cycle that wasn't visible at validation time. 2. I3 — only write parentId / categoryIds when the admin intentionally changed them (after differs from before). Previously Pass 3 always wrote these fields, which could clobber Pass 2's reference rewrites when the admin's after happened to match the pre-rewrite value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BulkWriter's per-doc Update returns a *BulkWriterJob future; the actual write outcome is reported through job.Results() which the executor never consulted. Permission-denied or contention failures on individual ref rewrites were silently lost, then the executor proceeded to delete the merge source — a real data-loss path. Switch to fs.Batch() + Commit(), which surfaces aggregate per-batch errors and matches the existing pattern used in cmd/backfill-curricula. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec section 9 requires partial-failure jobs to land as StatusFailed. Previously every Apply outcome — including ones where Firestore writes errored — became StatusApplied with a SkippedOp note, which buried the problem in the audit trail. Now we distinguish: - Stale records (Pass 1 concurrency mismatch, mergeInto target missing) remain skipped without flipping job status. Admin can re-run. - Real Firestore errors during Pass 2 merge or Pass 3 update flip the job to StatusFailed with the joined error reasons in job.Error, while still recording each as a SkippedOp in appliedResult so the admin can see exactly what didn't apply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec section 13 calls for an emulator-backed test of the merge cascade; adding the headline scenario: - Seed 3 categories (keeper, dup1 with two children, plus a third id) - Seed a technique whose categoryIds contains [dup1, keeper] (verifies the 'target already present' dedupe path) - Apply a job with one approved delete proposal: dup1 -> keeper - Assert dup1 is gone, both children now have parentId=keeper, and the technique's categoryIds is [keeper] (rewritten + deduped) Test skips cleanly when FIRESTORE_EMULATOR_HOST is not set so unit-test runs in CI / on dev machines without the emulator still pass. Verified PASS against the local emulator (localhost:8181). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Self-contained writeup of remaining items from the post-review pass: - 6 important runtime bugs (I1, I2, I4, I5, I6, I7) - 2 spec-mandated integration test scenarios still missing (T1, T2) - 9 minor polish items (M1-M9) - Manual E2E smoke test against running emulators Each entry has file:line references, why it matters, the concrete fix, and a verification step so a future session can pick any item up cold. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 'go build .' default produces a 'skillhive-api' binary in backend/ which polluted the VCS view. Project convention (per CLAUDE.md) is for all Go binaries to live under backend/bin/ — that path is already gitignored. This adds a per-binary entry matching the existing pattern (/backend/playlist-import, /backend/yt-enrich) for the bare-module build path. Verified no tracked files contain executable content (git ls-files | file --mime returned nothing matching executable/binary types). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tells GitHub Linguist: - frontend/dist/* -> generated (excluded from stats) - frontend/public/* -> vendored (excluded from stats) - *.vue -> not detectable (excluded from stats) Vue SFCs are the bulk of the line count in this repo, but the actual engineering work is split across Go (backend), TypeScript (frontend logic), and Vue templates. Excluding .vue lets the GitHub profile show the language mix that reflects the work, not just the SFC bytes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds support for the runtime API keys the cleanup admin and enrichment pipeline need, without breaking existing prod deployments. Changes: - env.example.sh: declare SECRET_GEMINI_API_KEY and SECRET_YOUTUBE_API_KEY - setup/03-setup-secrets.sh: two new interactive create_secret calls - deploy/backend.sh: maybe_add_secret() helper checks Secret Manager and only adds a --set-secrets binding if the secret exists. CORS remains unconditional. Verified by DRY_RUN against current prod state — when the new secrets don't exist, the deploy command is byte-identical to before; backend continues to degrade gracefully (main.go:46). - README.md: document the new optional secrets and the enable flow Operator workflow to enable a feature in prod: 1. run deploy/setup/03-setup-secrets.sh (interactive, prompts for key) 2. run deploy/deploy/backend.sh (auto-detects new secret, wires it) Compute SA already has project-wide secretmanager.secretAccessor (see setup/02-create-service-accounts.sh:65-71), so no IAM changes needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two preflight checks added to NewFirebaseClients to prevent the most
common cross-project credential leak when working with several GCP
projects locally.
1. Refuse to start when none of {FIREBASE_KEY_PATH, FIRESTORE_EMULATOR_HOST,
K_SERVICE} is set. Without an explicit credentials source the Firebase
Admin SDK would fall back to gcloud Application Default Credentials —
typically the wrong project for local dev. Cloud Run sets K_SERVICE
automatically so production is unaffected.
2. When FIREBASE_KEY_PATH is set, parse the JSON and verify project_id
matches GCP_PROJECT. Catches 'pointed at the wrong service-account
file' loudly at startup instead of letting the SDK happily authenticate
to the wrong project.
5 unit tests cover: project match, project mismatch, missing file,
missing project_id field, and the no-credentials-anywhere refusal.
No behavior change in prod (Cloud Run sets K_SERVICE) or in Docker
(docker-compose.yml sets FIRESTORE_EMULATOR_HOST). The new failure mode
fires only for 'go run .' from the command line without env config.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adding a tool used to mean either running 'go build ./cmd/foo' (which drops a 45MB binary at backend/ root and required a per-binary .gitignore entry) or 'go build -o bin/foo ./cmd/foo' (correct, but nobody remembered the -o flag). Result: 8 stale binaries in bin/ from February, plus 3 strays at backend/ root from later loose builds. This fixes the convention. backend/Makefile auto-discovers cmd/* subdirs and builds each into bin/<name>, plus 'server' from the root package. Adding a new tool at cmd/<name>/main.go needs no Makefile edit. cd backend && make build -> bin/server + bin/<every tool> cd backend && make tools -> just the tools cd backend && make clean -> wipe bin/ Root Makefile's 'make build' now delegates to the backend target (previously did 'go build ./...' which only compiled, never wrote binaries — this is now an actual build). Tested: 'rm -rf bin && make build' produces all 10 expected binaries in bin/ and nothing at backend/ root. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
No description provided.