Sam/skill marketplace - #7003
Closed
SamWang9803 wants to merge 535 commits into
Closed
Conversation
…ate (#78) The previous roll still failed: the box .env has no SMART_ROUTER_IMAGE, and docker-compose.prod.yml declares smart-router.image: ${SMART_ROUTER_IMAGE:?...}, so every `docker compose` invocation errored at interpolation before doing anything ("required variable SMART_ROUTER_IMAGE is missing a value"). The old ci-deploy.sh masked this as a benign "skip roll"; the new roll surfaced it as a hard failure. We only roll new-api, so export both image vars inline (a syntactically valid smart-router ref suffices — it's never pulled or recreated) and add --no-deps so the up only touches new-api. Smart-router and the other services are left as-is. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The Resources doc renderer (src/features/docs/components/doc-markdown.tsx) styles markdown entirely through Tailwind Typography `prose`/`prose-*` classes, but the plugin was never installed — so every one of those classes was a no-op and the guides rendered as flat, unstyled HTML (headings, code blocks, blockquotes, and tables all visually broken). Add `@tailwindcss/typography` (v0.5.20) and register it in the Tailwind v4 entry CSS via `@plugin`. Verified: `bun run build` exit 0 and `.prose*` rules now emit into the CSS bundle. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…-entry-point DR-73 add skill package analytics entry point
…y-guard Implement DR-80 runtime dependency guard
Load model_whitelist_snapshot from active SkillVersion at request time, override the client-supplied model with the first server-authoritative whitelist entry, inject instruction_template as system message, and strip all multi-turn history to enforce FR-G19 stateless single-turn. Empty whitelist → 500 SKILL_INTERNAL_ERROR; missing user message → 400 INVALID_REQUEST. Provider credentials and routing remain server-side throughout; package-supplied model hints are discarded entirely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
D5 (BLOCKER): move LoadAndApply to after applyAirbotixPolicy so kids-mode
model eligibility is checked against the client-supplied model name, not the
server-selected whitelist alias ("deeprouter-auto"). Virtual model aliases are
not in kids.EligibleModels and would cause a false 400 for every kids-session
skill request.
D4 (HIGH): skip channel-level SystemPrompt injection for skill relay requests.
instruction_template from SkillVersion is the authoritative system message
(DR-68); channel SystemPrompt must not prepend or override it.
D7 (cleanup): collapse dead errors.Is branch in loadSnapshot — both the
ErrRecordNotFound case and the else case returned identical ErrSkillInternalError.
Remove unused "errors" import.
D3 (documentation): add explicit V1 text-only comment in rewriteForSingleTurn
so the StringContent()-returns-"" behaviour for ContentPart arrays is a
documented limitation, not an invisible gap.
All 12 internal/... packages pass; relay/ passes (19+13 tests).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
applySystemPromptIfNeeded (Responses path) lacked the D4 guard — fixed by adding skillrelay.Get(c) check at function entry, mirroring the guard already in compatible_handler.go's conversion path. New unit tests (executor_test.go, +11 tests): - parseModelWhitelist: valid array, empty array, malformed JSON, zero-length raw - rewriteForSingleTurn: empty template allowed, assistant-only → ErrInvalidRequest, ContentPart text+image → text extracted (V1 documented), pure-image → ErrInvalidRequest (V1 text-only) - loadSnapshot: malformed whitelist_snapshot JSON → ErrSkillInternalError New integration tests (compatible_handler_skill_test.go, +4 tests): - TestTextHelper_SkillRelay_DR68_EmptyWhitelist_Returns500 - TestTextHelper_SkillRelay_DR68_NoUserMessage_Returns400 - TestApplySystemPromptIfNeeded_SkippedForSkillRelay (D4 Responses path guard) - TestApplySystemPromptIfNeeded_InjectsForNonSkillRelay (regression guard) All 12 internal/... packages pass; relay/ passes (45 total tests). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
prepareSkillRelayForDistribution runs in Distribute() before token model limits, smart-router, and channel selection. It unmarshal+resolve+LoadAndApply the SkillVersion snapshot and replaceReusableRequestBody replaces the raw BodyStorage so every downstream reader (model-limit check, smart-router sidecar, channel picker, TextHelper) sees only the server-selected model and the whitelist-constructed payload — never the client or package-supplied model, system prompt, history, or routing hints. - middleware/distributor.go: prepareSkillRelayForDistribution + replaceReusableRequestBody - middleware/distributor_skill_test.go: new tests — smart-router gets server snapshot model, client model never visible to downstream, package identity can't steer routing - Accompanying gofmt formatting: executor.go, compatible_handler.go, context.go, webhook.go, webhook_extra_test.go, smart_router_client/client.go Security invariants enforced end-to-end: - Package/client model → dropped before routing - smart-router only sees server whitelist model - provider payload = instruction_template + last user message only Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
compatible_handler.go lines 101-126: the two-path description was inaccurate. The comment implied applyAirbotixPolicy always sees the client model (D5 intent), but in the Distribute path prepareSkillRelayForDistribution has already called LoadAndApply and replaced the body, so policy sees the server whitelist model. Update comment to describe both paths accurately and note that kids-session model filtering against virtual aliases is V1 out-of-scope per DR-68 PRD. compatible_handler_skill_test.go: apply gofmt alignment (Model/Messages/Deeprouter struct literal fields). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Scan packages individually (not as a batch) so a single package failure doesn't silently truncate the test list; collect output in mktemp with trap cleanup; export GOCACHE to a local path for consistent CI behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
146 tests PASS / 0 FAIL across: internal/skill/relay (executor, loadSnapshot, selectModel, rewriteForSingleTurn) middleware (prepareSkillRelayForDistribution, replaceReusableRequestBody) relay (TextHelper skill paths, D4 SystemPrompt guards, kids-mode matrix) Command: go test -count=1 -p 1 -v ./internal/skill/relay/ ./middleware/ ./relay/ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
executor.go: replace direct encoding/json.Unmarshal with common.Unmarshal (common/json.go wrapper) per AGENTS.md Rule 1; drop encoding/json import. compatible_handler.go: guard TextHelper's LoadAndApply call with skillCtx.SkillVersionID == "". When SkillVersionID is already set, Distribute's prepareSkillRelayForDistribution already loaded the snapshot and rewrote the body; re-loading here would be a TOCTOU race — if active_version_id changes between Distribute and TextHelper, the provider payload could be rebuilt from a different snapshot than channel selection used, breaking the server-authoritative routing invariant. Skip the reload in that path; only load for the direct-TextHelper path (unit tests / callers that bypass Distribute), where SkillVersionID is still empty. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pliance, add tests
TOCTOU (root cause fixed):
compatible_handler.go — Resolve block now checks for existing SkillVersionID.
If set (Distribute path), reuse the pinned context and only update entry_point;
do NOT call Resolve again. A second Resolve returns a fresh zero-SkillVersionID
context; skillrelay.Set would overwrite the pin, making the SkillVersionID guard
always-true and triggering a second LoadAndApply against a potentially different
active_version_id — breaking server-authoritative routing.
Error path ordering (distributor.go replaceReusableRequestBody):
Seek new storage before closing old storage. If Seek fails, old storage remains
valid so any retry/panic-recovery path is not left with a closed BodyStorage.
Rule 1 — encoding/json replaced with common wrappers in all three test files:
executor_test.go: json.Marshal → common.Marshal; json.RawMessage → []byte
compatible_handler_skill_test.go: json.Marshal → common.Marshal
distributor_skill_test.go: json.Marshal → common.Marshal;
json.NewEncoder.Encode → common.Marshal + w.Write
Rule 2 — cross-DB SQL compatibility (executor_test.go):
datetime('now') (SQLite-only) → CURRENT_TIMESTAMP (SQL standard, all three DBs)
TestGap — prepareSkillRelayForDistribution error branches (distributor_skill_test.go):
+ TestPrepareSkillRelay_NilModelRequest_ReturnsEmpty
+ TestPrepareSkillRelay_NonChatPath_ReturnsEmpty
+ TestPrepareSkillRelay_NoSkillID_ReturnsEmpty
+ TestPrepareSkillRelay_UnknownSkillID_ReturnsError
+ TestPrepareSkillRelay_EmptyWhitelist_ReturnsInternalError
+ TestPrepareSkillRelay_NoUserMessage_ReturnsInvalidRequest
+ TestPrepareSkillRelay_TOCTOU_SkillVersionIDPinned (regression guard)
prepareSkillRelayForDistribution coverage: 68.4% → 89.5%
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ns deep-copy - Rule 8: extract prepareSkillRelayForDistribution + replaceReusableRequestBody from middleware/distributor.go into new middleware/skill_distributor.go so upstream cherry-picks to distributor.go do not conflict with skill-relay code - TOCTOU test fix: old TestPrepareSkillRelay_TOCTOU_SkillVersionIDPinned called prepareSkillRelayForDistribution twice against the same DB row — passed by coincidence, not because the guard fired. Replaced with TestTextHelper_SkillRelay_TOCTOU_PinnedVersionIDPreserved which pre-seeds a distinct pinnedID on the gin context and calls TextHelper, asserting that the Resolve block does NOT overwrite the already-pinned SkillVersionID - rewriteForSingleTurn: deep-copy StreamOptions (pointer, not aliased value) to prevent future in-place mutations from affecting the caller's original request; add TODO(DR-67) comment explaining intentional MaxTokens omission - compatible_handler.go direct path: add explicit skillrelay.Set after LoadAndApply to make the SkillVersionID update safe against any future Get-by-value refactor - Add kids-mode virtual alias TODO and replaceReusableRequestBody typed-nil comment - Update dr68-unit-regression.txt (154 PASS / 0 FAIL) with per-function coverage; add Coverage comments to all three test file headers per review guidance Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix formatting flagged by CI gofmt check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lection feat(DR-68): server-side skill routing with server-authoritative model selection
新增 docs/tasks/dr76-ops-overview-dashboard-prd.md（PRD-first, Rule 11）。 新增 features/skill-analytics/：types.ts 定義 DR-75 API 合約、api.ts 封裝 GET /api/v1/admin/skill-analytics/overview、metric-card.tsx（loading/no-data/tracking-failure 三態）、date-range-control.tsx（24h/7d/30d 預設，custom P1 disabled）、index.tsx 主頁面。 新增路由 _authenticated/skill-analytics/index.tsx（beforeLoad 守衛 ROLE.ADMIN，否則 /403）。 sidebar admin 組加入 Skill Analytics 導航項（BarChart2 icon）。 en.json + zh.json 新增 30 項 i18n key。 P0 卡片：WASU、Total Skill Runs、Skill Detail CTR、Enable Rate、First Use Rate、Repeat Use Rate、Block Rate、Top Block Reason、Revenue Attribution（charging_enabled=false 時隱藏）。 追蹤失敗橫幅（failed=橙色/delayed=黃色）；DR-75 未部署時顯示 error state。 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…verage)
安裝 Vitest + @testing-library/react 測試框架（首個前端測試套件）。
測試範圍：
types.test.ts — getDateRange（三個 preset、ISO8601 格式、start < end）
formatBlockReason（全部 6 個 BlockReason 值）
metric-card.test.tsx — loading/no-data/tracking-failed/normal 四態
date-range-control.test.tsx — preset 渲染、active variant、onChange、
disabled Custom range button
dashboard.integration.test.tsx — 21 個整合測試（vi.hoisted 確保 mock 在
factory 前初始化）：9 P0 卡片渲染、WASU/percent/USD 格式、Top Block Reason
標籤、tracking failed/delayed 橫幅、Revenue 條件隱藏、null 指標顯示 no-data
state、API error banner、日期切換觸發新 API 呼叫含範圍驗證
Production bug fixed during tests:
getDateRange(preset) 在 render 直接 new Date() 未 memoize，
每次 re-render 產生不同 queryKey，TanStack Query 無限 re-fetch。
Fix: useMemo(() => getDateRange(preset), [preset])
Coverage (src/features/skill-analytics/**):
Stmts 95.2% | Branch 98.2% | Funcs 93.3% | Lines 94.7%
api.ts 0% — intentionally mocked (expected)
types.ts branch 83.3% — unreachable ?? fallback in BlockReason union
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds per-file coverage breakdown from coverage-summary.json: index.tsx / metric-card.tsx / date-range-control.tsx all at 100%. api.ts 0% is intentional (mocked). types.ts branch 83.33% is an unreachable fallback in the BlockReason exhaustive map. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e-grant-isolation DR-98: Isolate referral grant from Skill purchase
…ion-migration Fix skill version instruction migration
…struction-migration-prd Mark skill version instruction migration shipped
…ompts-layout Fix Skill usage consent prompts and admin layout
Fix Skill usage dialog responsive width
…-analytics Add visual Skill usage analytics
…regression tests (#153) Seedance video models advertised in relay/channel/task/doubao ModelList were missing default model ratios, so any non-self-use deployment rejected them with 价格未配置. Add family-anchored default ratios (scaled off the 2.0 anchor by official Ark ¥/M-token pricing), a type-54 火山引擎 Seedance 视频 seed channel (reuses VOLCENGINE_API_KEY), and pricing-coverage regression tests wired into both CI gates. Task PRD included per AGENTS.md Rule 11. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
PR #153 merged (both CI gates green), production deploy auto-triggered. Records local e2e verification: all 6 Seedance models resolve pricing and reach Ark auth; pre-consumed quota refunded on failed submission. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix: reprice Seedance video from measured usage (per-call price, not token ratio) defaultModelPrice holds PER-CALL USD prices (quota = modelPrice * QuotaPerUnit; the task path keeps the pre-charge because BaseBilling.AdjustBillingOnComplete returns 0). The previous Seedance values were derived from the official ¥/M-token unit price as though this map held per-token ratios, underpricing every clip. Recalibrated from a real Ark task: a 5s 1080p Seedance-2.0 clip is 108,900 completion tokens, so cost is $0.69 and the old $0.15 charge lost $0.54 per video. New prices are measured cost x ~1.45. The residual exposure (flat price vs per-token upstream on longer clips) is documented in code and in the PRD, with the token-ratio migration named as the real fix if long clips take off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: add the dated Ark model ids that real calls dispatch GET /api/v3/models on a live Ark account returns DATED ids (doubao-seedream-5-0-260128), while the repo only carried undated aliases (doubao-seedream-5.0), so a real call hit 价格未配置. Adds the six ids present on that account. Two Seedance tiers have no published ¥/M — priced between the tiers above and below them and marked as estimates, erring toward over-charging rather than losing money. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…t billing (#156) * feat(models): refresh model catalogue to 2026-08 + fix Claude 5 output billing Anthropic's Claude 5 generation (Opus 5 / Sonnet 5 / Fable 5 / Mythos 5) was missing from every channel list, and — more seriously — none of the "-5" IDs matched a branch in getHardcodedCompletionModelRatio (which only knew claude-3 and claude-*-4). They fell through to a 1x completion ratio, i.e. output tokens billed at the input price for a family whose output costs 5x input. Same class of bug on GPT-5.6, which inherited the generic gpt-5 rule (8x) instead of 6x. Catalogue refresh across relay/channel/*/constants.go: - Anthropic: Claude 5 family (+ Opus 5 effort tiers), claude-sonnet-4-6 - OpenAI: GPT-5.5, GPT-5.6 (sol/terra/luna/cyber), realtime-2.x, transcribe - Gemini: 3.5 / 3.6 / 3.7 Flash, flash-lite, omni, deep-research - xAI: grok-4.6 / 4.5 / 4.3 / 4.20 / build-0.1 - Qwen: qwen3.7-* and qwen3.8-max - Kimi: kimi-k3, kimi-k2.7-code(-highspeed), k2.6 - GLM: glm-5.x moved to the v4 channel (zhipu_4v); relay/channel/zhipu keeps chatglm_* because it targets the retired v3 endpoint - Doubao: Seed 2.0; DeepSeek / Perplexity / Mistral / MiniMax synced scripts/seed-models/channels.yaml now seeds current models only and drops IDs retired upstream (all of Claude 3.x, deepseek-chat/reasoner, dall-e-*, llama-3-sonar-*, gemini-1.5-*). Two test_model values were pointing at retired IDs, so channel health checks could never pass. New guard TestSeededModelsArePriced walks the seed file and fails if an enabled channel seeds a model with no price — it surfaced 11 pre-existing gaps, now in a documented shrink-only allowlist. Priced Perplexity Sonar, gemini-2.5-flash-lite and omni-moderation-* along the way. kids_mode whitelist gains claude-haiku-4-5 (successor to the retired claude-3-5-haiku) with an assertion that frontier models stay out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NdKYxR4pQUXsJoWc1aUg9m * feat(volcengine): add Seedance 2.5 + Ark-hosted DeepSeek Both reach us through the same VolcEngine Ark key. Seedance 2.5 (doubao-seedance-2-5-260628, 2026-07-31) is deliberately NOT added to defaultModelPrice. That map holds flat per-call prices calibrated against a measured ~5s clip; 2.5 generates up to 30s per segment, so duration alone swings cost 6x and a flat charge loses money on every long generation — exactly the exposure the 2026-07-25 repricing note flagged. It goes in defaultModelRatio instead ($10.85/1M no-video-input rate → 5.425). Because it has a ratio and no per-call price, settleTaskBillingOnComplete re-settles from the task's actual TotalTokens. The $6.51/1M video-input rate rides the existing videoInputRatioMap discount (0.60), so the ratio has to stay the higher no-video figure. This failure mode is silent — a per-call price still bills, just too little — so TestSeedance25IsRatioBilledNotPerCall asserts the absence of a per-call price, the exact ratio, and the discount entry. DeepSeek on Ark: undated aliases (deepseek-v4-pro / -flash, which resolve on the Coding and Agent Plan endpoints) plus the dated ids I could verify against VolcEngine's docs (deepseek-v3-1-250821, deepseek-r1-250120), with ratios and completion ratios for the dated pair. Ark's dated V4 ids are not published anywhere I could confirm, so they are not guessed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NdKYxR4pQUXsJoWc1aUg9m --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The creator marketplace needs a schema that can express ownership, a review
workflow, prompt variables, per-call billing and ratings. This PRD is the
prerequisite for P2-P7 and settles three things the task card got wrong.
- D1: the user-rating table is skill_ratings, not skill_reviews. The latter is
already spec'd as the ops moderation queue and is where enums.ReviewStatus
comes from. Building ratings on it and reusing that enum would make every
rating summary read zero forever, silently, because the reader filters
status IN ('approved','published').
- D2: SQLite cannot ALTER an existing CHECK. Production is PostgreSQL and dev
SQLite databases are disposable, so the degradation is accepted and a
log-only boot probe tells the developer to delete one-api.db.
- D3: skills.status gains only submitted/sandbox/pending_launch. The
pending_review and suspended that downstream cards mention map onto
submitted+review_status and deprecated.
Also records three migration hazards found while reading the code: a new
check: struct tag on an existing table crashes the SQLite migrator at boot,
a CHECK expression edit without a drop hook is a permanent silent no-op on
existing PG/MySQL, and skills vs skill_versions take completely different
add-column paths.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Module3 P1, steps 1-2. Purely additive: nothing writes the new status values yet, so this is safe to ship ahead of P2-P7. - enums.SkillStatus gains submitted/sandbox/pending_launch, declared in lifecycle order. The test's invalid list now asserts pending_review and suspended are NOT statuses, turning the PRD's D3 mapping into something a future change trips over rather than a paragraph nobody reads. - skills gains 8 columns for ownership and the review trail. None of them carry a check: struct tag - a new constraint name against an existing table makes gorm call CreateConstraint, and the glebarez/sqlite migrator then rebuilds a table whose DDL contains IN(...) and fails with 'invalid DDL, unbalanced brackets'. Their CHECKs go in migrate.go's raw-DDL slice instead (PG/MySQL only). See PRD M1. - chk_skills_status is widened in the struct tag; the constraint NAME is unchanged, so a fresh database gets the new expression while an existing one short-circuits on HasConstraint. Existing PG/MySQL need the drop hook that lands with step 3. TestSkillStatusCheckTagCoversEveryEnumValue is new and covers a real gap: TestEnumDBValues_MatchCheckConstraints only compares each constant against a hardcoded literal and never reads the constraint, so adding an enum value without widening the CHECK passes it silently. The new guard reflects on the struct tag and was red-green checked by removing 'sandbox' from the expression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tabases Module3 P1, steps 3-4. Still purely additive - nothing writes the new status values yet. Two mechanisms here are the ones that fail silently if omitted: - refreshSkillsStatusConstraint. The apply loop in migrateSkillsConstraints skips any constraint whose NAME already exists, and widening the status expression did not change the name. Without an explicit drop, every existing PostgreSQL/MySQL database keeps the old four-value CHECK forever: no error, no failing test, just a status the application can never write. Written dialect-explicit (PG DROP CONSTRAINT IF EXISTS vs MySQL DROP CHECK) rather than via Migrator().DropConstraint, which infers the type from a struct tag the P1 columns deliberately do not have. - migrateSkillsCreatorColumns runs BEFORE AutoMigrate, so the columns and the source backfill land even when AutoMigrate later fails - which it does on SQLite, see below. chk_skills_source, chk_skills_review_status and chk_skills_creator_has_creator_id exist only in the raw-DDL slice, never as struct tags: a new constraint name against an existing table sends the glebarez/sqlite migrator into a table rebuild that fails at boot. The cost is that those three do not exist on SQLite at all; enums.Valid() and the SkillSource* constants are the gate there. skill_versions gains variables_schema and minhash_signature through the DR-93 three-site dance: struct, both hand-written CREATE TABLEs, and a backfilling add-column helper called on either side of AutoMigrate on the PG path. Also documents a PRE-EXISTING bug found while verifying this: a SQLite database cannot be migrated twice, so it fails to boot a second time (invalid DDL, unbalanced brackets). Verified on e1c0d12 both in-process and across a genuine close-and-reopen, so it is not a gorm schema-cache artifact. Production runs PostgreSQL and is unaffected; make dev users have to delete one-api.db. TestMigrateSkills_SQLite_RestartIsBroken asserts the current failure so it goes red when the driver is fixed. Tracked separately. The relay fixture needed variables_schema added to its raw INSERT - the expected consequence of a NOT NULL column, and the reason the whole suite gets run rather than just the touched package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…or invite
Module3 P1, steps 5-7. Two new tables and one column; nothing writes to any of
them yet.
skill_calls is the billing ledger, deliberately separate from
skill_usage_events. That table is analytics - losing a row costs a point on a
chart. This one answers 'what do we owe this creator this month' and has to
survive an audit, so it carries a request_id unique index as the
double-billing guard. request_id is *string rather than a defaulted empty
string so rows without one (backfills, internal calls) can coexist under that
index on every engine. Amounts are quota units, not cents: the commission is
credited to users.quota, an int of quota units, so cents would force a lossy
conversion at the moment money changes hands. No foreign keys, for the reason
skill_usage_events has none - an append-only ledger must outlive a hard delete
of the skill it refers to.
The rating table is skill_ratings, NOT skill_reviews. skill_reviews is already
spec'd as the ops moderation queue and is where enums.ReviewStatus comes from;
publicRatingSource() probes skill_ratings first anyway. Had ratings taken that
name and reused that enum, the reader's status IN ('approved','published')
filter would intersect open/assigned/escalated/resolved/reopened at exactly
zero values - every rating summary would read zero forever, with no error and
no failing test. Status defaults to approved because there is no
rating-moderation queue; defaulting to pending would ship a table whose every
row is invisible.
The DR-89 social-proof fixture hand-rolled a skill_reviews lookalike. With the
real table migrated those rows are never read, so it now builds rows through
the model. Its four assertions are unchanged, and the fixture can no longer
drift from the schema.
users.creator_invited_at is a timestamp rather than a bool: non-nil IS the
invite and it records when. Note for whoever wires the invite endpoint -
User.Update() uses Updates(struct), which skips zero values, so a nil pointer
never reaches the database; use an explicit Model().Update().
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 740 files, which is 440 over the limit of 300. To get a review, reduce the PR to 300 files or fewer by splitting it into smaller PRs or changing its base branch. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (30)
📒 Files selected for processing (740)
You can disable this status message by setting the 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 |
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
6 participants
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.
Opened against the wrong base repository by mistake (GitHub defaults to the
upstream parent for forks).
Closed — nothing to review here.