refactor: use shared @chittyos/schema scope-projector#103
Conversation
Replace inline fractal scope projection with thin adapter over the shared @chittyos/schema/scope-projector library. Reduces central-workflows.ts from 175 to 59 lines — the SQL upsert, status mapping, and type definitions now live in chittyschema and are reused across services. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
chittyfinance | e5904b7 | Apr 24 2026, 08:13 AM |
|
@coderabbitai review Please evaluate:
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe pull request introduces a new workspace dependency ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 docstrings
🧪 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 |
Code Review — PR #103: refactor: use shared @chittyos/schema scope-projectorOverview: This PR replaces 116 lines of inline scope-projection logic (SQL upsert, status mapping, type defs) with a thin adapter over a shared ✅ What's Working Well
🔴 Blockers1. "@chittyos/schema": "file:../../CHITTYFOUNDATION/chittyschema"This path is machine-local. Any environment where Recommended path forward (pick one):
This alone is a merge blocker if CI is currently passing only because the runner happens to have the path. 🟡 Issues Worth Addressing2. The old implementation explicitly put const metadata = JSON.stringify({ tenantId: projection.tenantId, ... });The new adapter does the same via spreading: metadata: { tenantId: projection.tenantId, ...(projection.metadata ?? {}) }This is fine if the shared 3. The old try {
c.executionCtx.waitUntil(promise);
} catch {
// Test environment / non-Workers runtime — swallow.
}The new code delegates this to 4. All three test plan items are unchecked The PR was opened with an incomplete test plan:
At minimum, 🟢 Minor / Nits5. The diff removes the JSDoc comments from each 6. Default const financeProjector = createScopeProjector('finance.chitty.cc', {
characterization: 'Project',
});This is a reasonable default for finance workflows, but Summary
Fix the 🤖 Reviewed with Claude Code |
There was a problem hiding this comment.
Pull request overview
Refactors ChittyFinance’s scope projection logic to delegate workflow→scope upserts to the shared @chittyos/schema/scope-projector, reducing local duplication and centralizing status/type mapping.
Changes:
- Replaced the inline Neon SQL upsert + status mapping in
central-workflows.tswith a thin adapter overcreateScopeProjector(...). - Re-exported shared scope types/constants from
@chittyos/schema/scope-projectorfor downstream usage. - Added
@chittyos/schemaas a dependency via a localfile:reference.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/lib/central-workflows.ts | Swaps in shared scope projector adapter and re-exports shared scope types/constants. |
| package.json | Adds @chittyos/schema dependency for the shared scope projector import. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function scopeLog( | ||
| c: { executionCtx: { waitUntil(p: Promise<unknown>): void } }, | ||
| projection: ScopeProjection, | ||
| env: ScopeEnv, | ||
| ): void { | ||
| const promise = projectScope(projection, env); | ||
| try { | ||
| c.executionCtx.waitUntil(promise); | ||
| } catch { | ||
| // Test environment / non-Workers runtime — swallow. | ||
| } | ||
| financeProjector(c, env, { | ||
| externalId: projection.externalId, | ||
| scopeType: projection.scopeType, | ||
| characterization: projection.characterization, | ||
| title: projection.title, | ||
| summary: projection.summary, | ||
| localStatus: projection.localStatus, | ||
| statusReason: projection.statusReason, | ||
| metadata: { | ||
| tenantId: projection.tenantId, | ||
| ...(projection.metadata ?? {}), | ||
| }, | ||
| }); |
| "test:run": "vitest run" | ||
| }, | ||
| "dependencies": { | ||
| "@chittyos/schema": "file:../../CHITTYFOUNDATION/chittyschema", |
![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: e5904b70bb
ℹ️ 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".
| "test:run": "vitest run" | ||
| }, | ||
| "dependencies": { | ||
| "@chittyos/schema": "file:../../CHITTYFOUNDATION/chittyschema", |
There was a problem hiding this comment.
The new dependency points to file:../../CHITTYFOUNDATION/chittyschema, which resolves outside this repository and is not present in a normal checkout, so installs fail unless a separate sibling repo is manually mounted. In this repo’s checked workflow (.github/workflows/security-gates.yml), only this repo is checked out before dependency install, so this change breaks automated installs/build validation in CI and for fresh clones.
Useful? React with 👍 / 👎.
| "test:run": "vitest run" | ||
| }, | ||
| "dependencies": { | ||
| "@chittyos/schema": "file:../../CHITTYFOUNDATION/chittyschema", |
There was a problem hiding this comment.
A direct dependency was added in package.json without updating pnpm-lock.yaml, which causes pnpm install --frozen-lockfile to fail due to lockfile/specifier mismatch. Since the security workflow runs frozen-lockfile installs, this commit will fail dependency audit/build gating until the lockfile is updated in the same change.
Useful? React with 👍 / 👎.
Adds the SERVICE_SCOPE_DB hyperdrive binding (id 89158b50e55a4d5d9279b5d5c890ea7b) to both root wrangler.jsonc and deploy/system-wrangler.jsonc. This is the service's own authoritative scopes Hyperdrive, paired with the existing CHITTYOS_CORE_DB binding for projection into ChittyOS-Core. Required by chittyschema/identity/scripts/validate-fractal-scopes.ts. The consumer code already exists in server/lib/central-workflows.ts via the @chittyos/schema/scope-projector adapter (PR #103) — without this binding the projector falls back to the SERVICE_SCOPE_DATABASE_URL env var. Validated: chittyschema validate-fractal-scopes.ts passes against both wrangler files (exit 0). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
central-workflows.tswith thin adapter over@chittyos/schema/scope-projector@chittyos/schemaas local file dependency (linked to../../CHITTYFOUNDATION/chittyschema)Test plan
npm run checkpasses (type-check)public.scopestable)🤖 Generated with Claude Code
Summary by CodeRabbit