Add Phase 5C Daily Compaction and Profile Synthesis - #107
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 50 minutes and 40 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPhase 5C adds a deterministic daily memory compaction pipeline: versioned ChangesPhase 5C Daily Compaction Pipeline
Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant DailyDigestRoute
participant runDailyMemoryCompaction
participant upsertVersionedMemoryProfile
participant upsertOpenLoop
participant Supabase
Scheduler->>DailyDigestRoute: POST /api/memory/jobs/daily-digest (Bearer token)
DailyDigestRoute->>DailyDigestRoute: validate token, check memoryDistillationEnabled, parse body
DailyDigestRoute->>runDailyMemoryCompaction: call with scope (user_id, namespace, since, dry_run)
runDailyMemoryCompaction->>Supabase: fetch memory_events, capture_candidates, feedback_events
runDailyMemoryCompaction->>upsertVersionedMemoryProfile: upsert each profile input
upsertVersionedMemoryProfile->>Supabase: supersede active row, insert new versioned row
runDailyMemoryCompaction->>upsertOpenLoop: upsert each loop input
upsertOpenLoop->>Supabase: merge evidence, update or insert open_loop row
runDailyMemoryCompaction->>Supabase: supersede prior active daily pack, insert new active pack
runDailyMemoryCompaction-->>DailyDigestRoute: MemoryCompactionResult
DailyDigestRoute-->>Scheduler: JSON (ok, namespace, profiles, loops, pack, warnings, blockers)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
besfeng23
left a comment
besfeng23
left a comment
There was a problem hiding this comment.
Block merge until precision patch is added.
The Phase 5C direction is right, but current profile synthesis can pollute durable memory.
Project profiles currently match either explicit project name OR generic technical terms like GitHub/PR/Vercel/Supabase/build/CI. That means one Pandora Memory PR source can create/update PLP, Hatid, Speedcash, SpeedyPay, RetargetOS, AI GrowthOS, and Red-Apple profiles even when those projects are not mentioned.
Same issue with relationship/person profiles: generic “relationship loop” language can attach evidence to Patty without an explicit Patty mention.
Required before merge:
- Named project/person profiles require explicit subject-name hits.
- Generic technical/deployment evidence goes into a global technical_activity_profile.
- Generic relationship/emotional evidence goes into a global relationship_loop_profile.
- Add negative tests proving unrelated project/person profiles are not created.
After that patch, this is mergeable.
![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: ffaa19da83
ℹ️ 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".
| import { runDailyMemoryCompaction } from "@/lib/services/memory-compaction-service"; | ||
| const schema=z.object({namespace:z.enum(["real_life","au"]),since:z.string().datetime().optional(),dry_run:z.boolean().optional(),user_id:z.string().min(1).optional()}); | ||
| function bearer(request:NextRequest){const [scheme,token]=(request.headers.get("authorization")??"").split(" "); return scheme?.toLowerCase()==="bearer"?token:undefined;} | ||
| export async function POST(request:NextRequest){const token=process.env.PANDORA_INTERNAL_JOB_TOKEN; if(!token||bearer(request)!==token)return NextResponse.json({ok:false,blockers:["internal_job_token_required"]},{status:401}); const runtime=resolvePandoraRuntimeSafetyConfig(); if(!runtime.config.memoryDistillationEnabled)return NextResponse.json({ok:false,blockers:["memoryDistillationEnabled_disabled"],warnings:[],next_step:"Set PANDORA_ENABLE_MEMORY_DISTILLATION=true in a reviewed environment."},{status:403}); const parsed=schema.safeParse(await request.json().catch(()=>({}))); if(!parsed.success)return NextResponse.json({ok:false,blockers:["invalid_request"],issues:parsed.error.flatten()},{status:400}); const user_id=parsed.data.user_id??process.env.PANDORA_MEMORY_BRIDGE_USER_ID; if(!user_id)return NextResponse.json({ok:false,blockers:["job_user_id_required"],next_step:"Send user_id with the internal token or configure PANDORA_MEMORY_BRIDGE_USER_ID."},{status:400}); const result=await runDailyMemoryCompaction(createSupabaseBridgeAdminClient() as never,{user_id,namespace:parsed.data.namespace,since:parsed.data.since,dry_run:parsed.data.dry_run}); return NextResponse.json({ok:result.ok,namespace:result.namespace,dry_run:result.dry_run,daily_pack:result.daily_pack,updated_profiles:result.updated_profiles,opened_or_updated_loops:result.opened_or_updated_loops,warnings:result.warnings,blockers:result.blockers,next_step:result.next_step});} |
There was a problem hiding this comment.
When the internal job token is accepted, this route falls back to parsed.data.user_id and then uses the bridge admin client, so any caller with that token can choose another user's id and write profiles, open loops, and context packs for that user. That bypasses the AGENTS.md requirement that memory writes be tied to server-derived or RLS-derived identity; use only a server-side configured user/session mapping instead of a request-body user_id.
Useful? React with 👍 / 👎.
| confidence: input.confidence ?? previous?.confidence ?? 0.6, | ||
| status: "active", | ||
| version, | ||
| supersedes_profile_id: previous?.id, |
There was a problem hiding this comment.
On any subsequent compaction for an existing active profile, previous?.id is set and the inserted row includes supersedes_profile_id, but the memory_profiles migration only defines columns through updated_at and has no supersedes_profile_id column; line 56 also writes a non-existent superseded_at. Supabase will reject these requests, so profile versioning fails as soon as a profile already exists.
Useful? React with 👍 / 👎.
| }; | ||
| if (input.dry_run) return { ok: true, dry_run: true, profile: row, previous_profile: previous, blockers: [], warnings: [] }; | ||
| if (previous?.id) await client.from("memory_profiles").update({ status: "superseded", superseded_at: new Date().toISOString(), updated_at: new Date().toISOString() }).eq("id", previous.id).eq("user_id", input.user_id).eq("namespace", input.namespace).select("*").single(); | ||
| const inserted = await client.from("memory_profiles").insert(row).select("*").single(); |
There was a problem hiding this comment.
This direct insert creates durable profile versions without writing any audit_logs entry, unlike existing memory write paths such as createContextPack and proposal persistence. In production compaction runs, profile changes will not be audit-backed, violating the project persistence rules and leaving readback unable to prove what changed.
Useful? React with 👍 / 👎.
| add("operating_profile","global","Global operating profile",sources.filter(x=>has(textOf(x),["I prefer","from now on","always","don't","dont","be blunt","be concise","don't overpraise","check repo first","analyze automatically","maximize Codex task"])),"preferences"); | ||
| add("style_profile","global","Global style profile",sources.filter(x=>has(textOf(x),["tone","style","format","writing","prompt style","direct","Taglish","cinematic"])),"preferences"); | ||
| add("risk_profile","global","Global risk profile",sources.filter(x=>has(textOf(x),["gambling","money risk","reputation risk","avoidance","fantasy-vs-execution","legal risk","deployment claim not verified","fake progress","risk"])),"risks"); | ||
| for(const p of projects){add("project_profile",subject(p),`${p} project profile`,sources.filter(x=>has(textOf(x),[p])||has(textOf(x),["GitHub","PR","merge","Vercel","Supabase","migration","deployment","build","CI","repo"])),"facts"); add("technical_deployment_profile",subject(p),`${p} deployment profile`,sources.filter(x=>has(textOf(x),[p])&&has(textOf(x),["deployed","failed","pending","merged","blocked","production","Vercel","GitHub Actions","CodeRabbit","Supabase migration"])),"facts");} |
There was a problem hiding this comment.
For every configured project, this predicate also accepts generic repo/deployment words even when the source text does not mention p, so one "GitHub PR" or "Vercel" memory is copied into PLP, Hatid, Speedcash, and all other project profiles. That pollutes unrelated durable project profiles during compaction; require the project name or explicit project metadata before writing a per-project profile.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (1)
tests/phase5c-memory-compaction.test.ts (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract and type the mock client for readability and maintainability.
The inline
clientfactory spans an extremely long line with deeply nested object literals and mixedanytypes. Extract it into a well-named helper with explicit return types, or at least split across multiple lines with intermediate variables. This improves debuggability when Supabase contract changes break the mock.🤖 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 `@tests/phase5c-memory-compaction.test.ts` at line 6, The inline mock client in client is overly dense and relies on broad any types, making the test hard to read and maintain. Extract the factory into a clearly named helper with explicit return types (or split it into multiple intermediate variables and multiline objects), and replace the mixed any usage with typed shapes for rows and the query API so changes to the Supabase contract are easier to catch.
🤖 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 `@app/admin/memory/compaction/page.tsx`:
- Around line 1-2: The MemoryCompactionAdminPage is currently rendered
statically, which can freeze the runtime safety gate data and the since
timestamp at build time. Add a dynamic rendering export alongside the existing
MemoryCompactionAdminPage component so this admin page always evaluates
resolvePandoraRuntimeSafetyConfig at request time and keeps its runtime checks
current.
In `@app/api/memory/jobs/daily-digest/route.ts`:
- Line 8: The POST handler in daily-digest/route.ts always responds with HTTP
200 even when runDailyMemoryCompaction returns ok:false, so update the final
NextResponse.json call to set an error status whenever the result indicates
failure. Use the existing runDailyMemoryCompaction result fields (ok, blockers,
next_step) to choose a non-2xx status for failed compactions while keeping
success responses unchanged.
- Line 6: The daily digest input schema rejects ISO timestamps with timezone
offsets because the `schema` definition uses `z.string().datetime()` for
`since`. Update the `since` validator in the `route.ts` schema to accept
offset-aware ISO datetimes by using the datetime option with offsets enabled,
keeping the existing optional behavior intact.
In `@lib/services/memory-compaction-service.ts`:
- Line 17: The list() helper in memory-compaction-service is applying
since/status filtering after limit(100), which can skip eligible rows on busy
tables. Move the source filters into the query built from client.from(table)
before the limit is applied, using the same MemoryCompactionScope fields and
statuses constraints, and keep the existing blocked_secret/textOf filtering as
needed for any remaining in-memory checks.
- Line 30: The daily pack replacement flow in the compaction service ignores
failures from the supersede step, which can allow multiple active packs to
exist. Update the logic around the
client.from("memory_context_packs").update(...) call in the compaction path to
inspect its result and stop/record a warning on any real failure, only
proceeding to the insert when the update truly affected no rows. Keep the fix
localized to the daily-pack handling branch that builds pack and writes to
memory_context_packs.
- Around line 18-23: The subject-specific profile builders in profileInputs are
over-broad: the project_profile and technical_deployment_profile entries, as
well as the people_profile/relationship_loop_profile entries, are collecting
generic sources that can belong to any subject. Tighten the source filters so
each add(...) call only includes evidence when the project or person name
actually matches the current subject, and keep the generic GitHub/CI/deployment
or relationship-loop terms as secondary qualifiers rather than standalone
matches.
- Around line 28-30: The compaction flow in memory-compaction-service is masking
failed upserts by pushing r.blockers into warnings and still returning ok:true.
Update the profile and loop loops to preserve blockers in base.blockers (not
warnings), and if any required upsert fails, stop before buildDailyContextPack
and the memory_context_packs write so the function returns failure instead of a
successful daily pack.
In `@lib/services/memory-open-loop-service.ts`:
- Around line 9-14: The active-loop lookup in memory-open-loop-service is
applying the limit before checking status, which can miss an existing open or
acknowledged loop and cause duplicates. Update the existing query logic in the
service function that builds existing/active from
client.from("memory_open_loops") so it filters for open/acknowledged records
first, then applies ordering/limit only if needed; keep the same merge/update
path when an active loop is found.
In `@lib/services/memory-profile-service.ts`:
- Around line 56-58: The rollover logic in memory-profile-service’s profile
write path is not atomic because the existing record is superseded before the
replacement insert and the update result is ignored. Update the flow around the
existing client.from("memory_profiles") update/insert sequence so the supersede
result is checked before continuing, or move both steps into a single
transactional RPC/helper on the database side to ensure only one active profile
can exist. If you keep the current structure, handle any update failure by
returning an error before attempting the insert, and keep the existing insert
failure handling in the same profile write function.
In `@lib/services/memory-session-digest-service.ts`:
- Line 12: The master pack is being built directly from raw memory_events in
createMasterPackFromProfiles, so sensitive source text can be persisted. Update
createMasterPackFromProfiles to filter out blocked-secret rows and redact event
text before passing the events into buildMasterContextPack, matching the
sanitization used by the daily compaction flow.
- Line 12: The createMasterPackFromProfiles flow is inserting a new active
master pack without retiring the previous one, which can leave multiple active
packs for the same user and namespace. Update createMasterPackFromProfiles to
supersede any existing active master pack for the same (user_id, namespace,
pack_type) before the insert, ideally in the same operation flow as the
memory_context_packs write. Use the createMasterPackFromProfiles function and
the memory_context_packs insert path as the anchors when applying the fix.
In `@tests/phase5c-memory-compaction.test.ts`:
- Line 10: The redaction check in runDailyMemoryCompaction is vacuous because
the seeded memory_events never include a blocked secret to remove. Update the
test to add a positive secret-containing event in the client fixture, then
assert that the resulting daily pack from runDailyMemoryCompaction does not
include that specific secret after cleaning, while keeping the existing
pack-type and profile assertions.
- Line 6: The mock client’s order helper in client is comparing every field as
strings, which breaks numeric ordering for version and can make limit(1) return
the wrong memory_profiles row. Update the comparator inside
client.from(...).order(...) to handle the version key numerically while
preserving the existing string-based fallback for other columns. Keep the
behavior consistent for ascending and descending so tests that exercise
upsertVersionedMemoryProfile and related memory profile queries stay
deterministic.
- Around line 16-18: The Phase 5C job endpoint test is mutating process.env
without cleanup and only covers failure cases. In
tests/phase5c-memory-compaction.test.ts, add setup/teardown around route imports
to save and restore PANDORA_INTERNAL_JOB_TOKEN and
PANDORA_ENABLE_MEMORY_DISTILLATION, then keep the current 401/403 assertions but
also add a success-path case for route.POST when the token is valid and memory
distillation is enabled. Use the same POST handler and route import pattern so
the added test exercises the real 200 response path.
---
Nitpick comments:
In `@tests/phase5c-memory-compaction.test.ts`:
- Line 6: The inline mock client in client is overly dense and relies on broad
any types, making the test hard to read and maintain. Extract the factory into a
clearly named helper with explicit return types (or split it into multiple
intermediate variables and multiline objects), and replace the mixed any usage
with typed shapes for rows and the query API so changes to the Supabase contract
are easier to catch.
🪄 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: a6cd965d-10c6-414a-b70e-a4d10f4ef496
📒 Files selected for processing (9)
app/admin/memory/compaction/page.tsxapp/api/memory/jobs/daily-digest/route.tsdocs/pandora-memory-autopilot.mddocs/pandora-memory-compaction.mdlib/services/memory-compaction-service.tslib/services/memory-open-loop-service.tslib/services/memory-profile-service.tslib/services/memory-session-digest-service.tstests/phase5c-memory-compaction.test.ts
| import { resolvePandoraRuntimeSafetyConfig } from "@/lib/config/pandora-runtime-safety-config"; | ||
| export default function MemoryCompactionAdminPage(){const r=resolvePandoraRuntimeSafetyConfig(); const safe=[ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Next.js 15 App Router page static rendering force-dynamic route segment config
💡 Result:
In Next.js 15, the dynamic route segment configuration option remains the primary mechanism to explicitly control the rendering mode of a page or layout [1][2]. To force a route to be rendered dynamically (on every request), you can export the following from your page.tsx, layout.tsx, or Route Handler: export const dynamic = 'force-dynamic'; Effects of 'force-dynamic': - It forces the entire route segment to be rendered at request time [2][3]. - It is equivalent to setting all fetch requests within that segment to { cache: 'no-store' } [2][4]. - It ensures the route is not pre-rendered at build time [5]. Distinction between Rendering and Caching: It is important to distinguish between server-side rendering (the generation of the page) and the Next.js Router Cache (a client-side cache) [4][6]. While export const dynamic = 'force-dynamic' forces the page to be rendered dynamically on the server for each request, the client-side Router Cache may still store the resulting page navigation to improve perceived performance [7][6]. If you observe that a page seems to be "cached" despite using force-dynamic, you may be encountering the client-side Router Cache [6]. You can control the duration of this cache in your next.config.js via the experimental.staleTimes configuration [7]. Additionally, in Next.js 15, you can opt into dynamic behavior at a more granular level by invoking dynamic APIs [8][3]. The connection() function from next/server can be used within an asynchronous function to signal that the execution should be dynamic, ensuring the route is rendered at request time [9]. Note: The dynamic configuration takes precedence over individual fetch options within the same segment [4]. If you need a more precise approach, prefer using { cache: 'no-store' } on specific fetch calls rather than forcing the entire route to be dynamic [4].
Citations:
- 1: https://nextjs.org/docs/app/api-reference/file-conventions/route-segment-config
- 2: https://nextjs.im/docs/15/app/api-reference/file-conventions/route-segment-config/
- 3: https://www.elevaseo.com/en/blog/performance/ssr-ssg-isr-comparison
- 4: https://dev.to/jtorchia/nextjs-app-router-caching-revalidate-dynamic-and-no-store-without-the-folklore-2db3
- 5: failed to build vercel/next.js#80404
- 6: https://stackoverflow.com/questions/77190252/nexts-page-tsx-export-force-dynamic-does-not-render-dynamically
- 7: https://nextjs.thisisme1228.com/docs/content/app/building-your-application/upgrading/version-15
- 8: https://nextjs.org/learn/dashboard-app/static-and-dynamic-rendering
- 9: https://stackoverflow.com/questions/78118372/running-a-custom-function-in-a-server-component-using-next-js-app-router-at-runt
Force this admin page to render dynamically. Add export const dynamic = "force-dynamic"; so the runtime safety gates and since timestamp aren’t frozen at build time.
Proposed fix
import { resolvePandoraRuntimeSafetyConfig } from "`@/lib/config/pandora-runtime-safety-config`";
+
+export const dynamic = "force-dynamic";
+
export default function MemoryCompactionAdminPage(){const r=resolvePandoraRuntimeSafetyConfig(); const safe=[📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { resolvePandoraRuntimeSafetyConfig } from "@/lib/config/pandora-runtime-safety-config"; | |
| export default function MemoryCompactionAdminPage(){const r=resolvePandoraRuntimeSafetyConfig(); const safe=[ | |
| import { resolvePandoraRuntimeSafetyConfig } from "`@/lib/config/pandora-runtime-safety-config`"; | |
| export const dynamic = "force-dynamic"; | |
| export default function MemoryCompactionAdminPage(){const r=resolvePandoraRuntimeSafetyConfig(); const safe=[ |
🤖 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 `@app/admin/memory/compaction/page.tsx` around lines 1 - 2, The
MemoryCompactionAdminPage is currently rendered statically, which can freeze the
runtime safety gate data and the since timestamp at build time. Add a dynamic
rendering export alongside the existing MemoryCompactionAdminPage component so
this admin page always evaluates resolvePandoraRuntimeSafetyConfig at request
time and keeps its runtime checks current.
| import { resolvePandoraRuntimeSafetyConfig } from "@/lib/config/pandora-runtime-safety-config"; | ||
| import { createSupabaseBridgeAdminClient } from "@/lib/supabase/bridge-admin"; | ||
| import { runDailyMemoryCompaction } from "@/lib/services/memory-compaction-service"; | ||
| const schema=z.object({namespace:z.enum(["real_life","au"]),since:z.string().datetime().optional(),dry_run:z.boolean().optional(),user_id:z.string().min(1).optional()}); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Zod 3.23.8 z.string().datetime offset true default
💡 Result:
In Zod 3.23.8, the default value for the offset option in z.string.datetime({ offset:... }) is false [1][2][3]. When calling z.string.datetime, Zod enforces ISO 8601 strings and, by default, strictly requires UTC format (usually ending in "Z") and prohibits timezone offsets [1][2][3]. Setting { offset: true } explicitly enables the validation to accept strings that include timezone offsets (e.g., +02:00) [1][2][3]. Summary of datetime validation behavior: - Default: No timezone offsets are allowed [1][3]. - Enabled: { offset: true } allows timezone offsets [1][2]. - Precision: Arbitrary sub-second decimal precision is supported by default [1][3]. - Unqualified/Local: To allow datetimes without a timezone or "Z" designator, you must use the { local: true } option, which was added in Zod 3.23.0 [4][5][6].
Citations:
- 1: https://zod.dev/api
- 2: https://npmx.dev/package/zod/v/3.23.8
- 3: https://github.com/colinhacks/zod/blob/42984bf92b93b468666f64d536016b1439f8bf9e/README.md
- 4: https://github.com/colinhacks/zod/releases/tag/v3.23.0
- 5: Datetime validation without offset should allow omission of Z zone designator colinhacks/zod#2385
- 6: https://v3.zod.dev/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate Zod version declarations and the target route
git ls-files | rg '(^|/)(package\.json|pnpm-lock\.yaml|package-lock\.json|yarn\.lock|bun\.lockb|app/api/memory/jobs/daily-digest/route\.ts)$'
printf '\n--- package.json mentions ---\n'
rg -n '"zod"|zod@' package.json **/package.json 2>/dev/null || true
printf '\n--- lockfile mentions ---\n'
rg -n 'zod' pnpm-lock.yaml package-lock.json yarn.lock bun.lockb 2>/dev/null || true
printf '\n--- route file ---\n'
cat -n app/api/memory/jobs/daily-digest/route.tsRepository: besfeng23/Memory
Length of output: 3179
Allow ISO since values with offsets
z.string().datetime() rejects offset timestamps by default, so inputs like 2026-06-28T10:00:00-07:00 fail here. If this field is meant to accept ISO datetimes, switch to datetime({ offset: true }).
Proposed fix
-const schema=z.object({namespace:z.enum(["real_life","au"]),since:z.string().datetime().optional(),dry_run:z.boolean().optional(),user_id:z.string().min(1).optional()});
+const schema=z.object({namespace:z.enum(["real_life","au"]),since:z.string().datetime({offset:true}).optional(),dry_run:z.boolean().optional(),user_id:z.string().min(1).optional()});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const schema=z.object({namespace:z.enum(["real_life","au"]),since:z.string().datetime().optional(),dry_run:z.boolean().optional(),user_id:z.string().min(1).optional()}); | |
| const schema=z.object({namespace:z.enum(["real_life","au"]),since:z.string().datetime({offset:true}).optional(),dry_run:z.boolean().optional(),user_id:z.string().min(1).optional()}); |
🤖 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 `@app/api/memory/jobs/daily-digest/route.ts` at line 6, The daily digest input
schema rejects ISO timestamps with timezone offsets because the `schema`
definition uses `z.string().datetime()` for `since`. Update the `since`
validator in the `route.ts` schema to accept offset-aware ISO datetimes by using
the datetime option with offsets enabled, keeping the existing optional behavior
intact.
| import { runDailyMemoryCompaction } from "@/lib/services/memory-compaction-service"; | ||
| const schema=z.object({namespace:z.enum(["real_life","au"]),since:z.string().datetime().optional(),dry_run:z.boolean().optional(),user_id:z.string().min(1).optional()}); | ||
| function bearer(request:NextRequest){const [scheme,token]=(request.headers.get("authorization")??"").split(" "); return scheme?.toLowerCase()==="bearer"?token:undefined;} | ||
| export async function POST(request:NextRequest){const token=process.env.PANDORA_INTERNAL_JOB_TOKEN; if(!token||bearer(request)!==token)return NextResponse.json({ok:false,blockers:["internal_job_token_required"]},{status:401}); const runtime=resolvePandoraRuntimeSafetyConfig(); if(!runtime.config.memoryDistillationEnabled)return NextResponse.json({ok:false,blockers:["memoryDistillationEnabled_disabled"],warnings:[],next_step:"Set PANDORA_ENABLE_MEMORY_DISTILLATION=true in a reviewed environment."},{status:403}); const parsed=schema.safeParse(await request.json().catch(()=>({}))); if(!parsed.success)return NextResponse.json({ok:false,blockers:["invalid_request"],issues:parsed.error.flatten()},{status:400}); const user_id=parsed.data.user_id??process.env.PANDORA_MEMORY_BRIDGE_USER_ID; if(!user_id)return NextResponse.json({ok:false,blockers:["job_user_id_required"],next_step:"Send user_id with the internal token or configure PANDORA_MEMORY_BRIDGE_USER_ID."},{status:400}); const result=await runDailyMemoryCompaction(createSupabaseBridgeAdminClient() as never,{user_id,namespace:parsed.data.namespace,since:parsed.data.since,dry_run:parsed.data.dry_run}); return NextResponse.json({ok:result.ok,namespace:result.namespace,dry_run:result.dry_run,daily_pack:result.daily_pack,updated_profiles:result.updated_profiles,opened_or_updated_loops:result.opened_or_updated_loops,warnings:result.warnings,blockers:result.blockers,next_step:result.next_step});} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Return a failure HTTP status when compaction fails.
Line 8 always returns default HTTP 200, even when runDailyMemoryCompaction returns ok:false with blockers such as compaction_failed. Job schedulers and alerts will treat failed compactions as successful.
Proposed fix
- const result=await runDailyMemoryCompaction(createSupabaseBridgeAdminClient() as never,{user_id,namespace:parsed.data.namespace,since:parsed.data.since,dry_run:parsed.data.dry_run}); return NextResponse.json({ok:result.ok,namespace:result.namespace,dry_run:result.dry_run,daily_pack:result.daily_pack,updated_profiles:result.updated_profiles,opened_or_updated_loops:result.opened_or_updated_loops,warnings:result.warnings,blockers:result.blockers,next_step:result.next_step});}
+ const result=await runDailyMemoryCompaction(createSupabaseBridgeAdminClient() as never,{user_id,namespace:parsed.data.namespace,since:parsed.data.since,dry_run:parsed.data.dry_run}); return NextResponse.json({ok:result.ok,namespace:result.namespace,dry_run:result.dry_run,daily_pack:result.daily_pack,updated_profiles:result.updated_profiles,opened_or_updated_loops:result.opened_or_updated_loops,warnings:result.warnings,blockers:result.blockers,next_step:result.next_step},{status:result.ok?200:500});}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function POST(request:NextRequest){const token=process.env.PANDORA_INTERNAL_JOB_TOKEN; if(!token||bearer(request)!==token)return NextResponse.json({ok:false,blockers:["internal_job_token_required"]},{status:401}); const runtime=resolvePandoraRuntimeSafetyConfig(); if(!runtime.config.memoryDistillationEnabled)return NextResponse.json({ok:false,blockers:["memoryDistillationEnabled_disabled"],warnings:[],next_step:"Set PANDORA_ENABLE_MEMORY_DISTILLATION=true in a reviewed environment."},{status:403}); const parsed=schema.safeParse(await request.json().catch(()=>({}))); if(!parsed.success)return NextResponse.json({ok:false,blockers:["invalid_request"],issues:parsed.error.flatten()},{status:400}); const user_id=parsed.data.user_id??process.env.PANDORA_MEMORY_BRIDGE_USER_ID; if(!user_id)return NextResponse.json({ok:false,blockers:["job_user_id_required"],next_step:"Send user_id with the internal token or configure PANDORA_MEMORY_BRIDGE_USER_ID."},{status:400}); const result=await runDailyMemoryCompaction(createSupabaseBridgeAdminClient() as never,{user_id,namespace:parsed.data.namespace,since:parsed.data.since,dry_run:parsed.data.dry_run}); return NextResponse.json({ok:result.ok,namespace:result.namespace,dry_run:result.dry_run,daily_pack:result.daily_pack,updated_profiles:result.updated_profiles,opened_or_updated_loops:result.opened_or_updated_loops,warnings:result.warnings,blockers:result.blockers,next_step:result.next_step});} | |
| export async function POST(request:NextRequest){const token=process.env.PANDORA_INTERNAL_JOB_TOKEN; if(!token||bearer(request)!==token)return NextResponse.json({ok:false,blockers:["internal_job_token_required"]},{status:401}); const runtime=resolvePandoraRuntimeSafetyConfig(); if(!runtime.config.memoryDistillationEnabled)return NextResponse.json({ok:false,blockers:["memoryDistillationEnabled_disabled"],warnings:[],next_step:"Set PANDORA_ENABLE_MEMORY_DISTILLATION=true in a reviewed environment."},{status:403}); const parsed=schema.safeParse(await request.json().catch(()=>({}))); if(!parsed.success)return NextResponse.json({ok:false,blockers:["invalid_request"],issues:parsed.error.flatten()},{status:400}); const user_id=parsed.data.user_id??process.env.PANDORA_MEMORY_BRIDGE_USER_ID; if(!user_id)return NextResponse.json({ok:false,blockers:["job_user_id_required"],next_step:"Send user_id with the internal token or configure PANDORA_MEMORY_BRIDGE_USER_ID."},{status:400}); const result=await runDailyMemoryCompaction(createSupabaseBridgeAdminClient() as never,{user_id,namespace:parsed.data.namespace,since:parsed.data.since,dry_run:parsed.data.dry_run}); return NextResponse.json({ok:result.ok,namespace:result.namespace,dry_run:result.dry_run,daily_pack:result.daily_pack,updated_profiles:result.updated_profiles,opened_or_updated_loops:result.opened_or_updated_loops,warnings:result.warnings,blockers:result.blockers,next_step:result.next_step},{status:result.ok?200:500});} |
🤖 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 `@app/api/memory/jobs/daily-digest/route.ts` at line 8, The POST handler in
daily-digest/route.ts always responds with HTTP 200 even when
runDailyMemoryCompaction returns ok:false, so update the final NextResponse.json
call to set an error status whenever the result indicates failure. Use the
existing runDailyMemoryCompaction result fields (ok, blockers, next_step) to
choose a non-2xx status for failed compactions while keeping success responses
unchanged.
| const has=(t:string,words:string[])=>words.some(w=>t.toLowerCase().includes(w.toLowerCase())); | ||
| const ref=(kind:string,x:any)=>({kind,id:x.id,source:x.source,source_ref:x.source_ref,created_at:x.created_at??x.reviewed_at}); | ||
| const subject=(s:string)=>s.toLowerCase().replace(/[^a-z0-9]+/g,"_").replace(/^_|_$/g,"")||"global"; | ||
| async function list(client:MemoryBridgeDbClient,table:string,input:MemoryCompactionScope,statuses?:string[]){const q:any=client.from(table).select("*").eq("user_id",input.user_id).eq("namespace",input.namespace).order("created_at",{ascending:false}).limit(100);const r=await (q as Promise<{data:any[]|null;error:{message:string}|null}>); if(r.error)throw new Error(`${table}: ${r.error.message}`); return (r.data??[]).filter(x=>(!input.since||String(x.created_at??x.reviewed_at??"")>=input.since!)&&(!statuses||statuses.includes(x.status))).filter(x=>x.status!=="blocked_secret"&&textOf(x)!==blocked);} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Apply source filters before the 100-row limit.
Line 17 fetches the latest 100 rows first, then filters by since and status in memory. Busy tables with many rejected/blocked/out-of-window rows can cause eligible records in the compaction window to be skipped. Push since/status filtering into the query before limit(100).
🤖 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 `@lib/services/memory-compaction-service.ts` at line 17, The list() helper in
memory-compaction-service is applying since/status filtering after limit(100),
which can skip eligible rows on busy tables. Move the source filters into the
query built from client.from(table) before the limit is applied, using the same
MemoryCompactionScope fields and statuses constraints, and keep the existing
blocked_secret/textOf filtering as needed for any remaining in-memory checks.
| if (previous?.id) await client.from("memory_profiles").update({ status: "superseded", superseded_at: new Date().toISOString(), updated_at: new Date().toISOString() }).eq("id", previous.id).eq("user_id", input.user_id).eq("namespace", input.namespace).select("*").single(); | ||
| const inserted = await client.from("memory_profiles").insert(row).select("*").single(); | ||
| if (inserted.error || !inserted.data) return { ok: false, dry_run: false, blockers: ["profile_write_failed"], warnings: [inserted.error?.message ?? "unknown profile write failure"], next_step: "Check memory_profiles schema and RLS." }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make profile version rollover atomic.
Line 56 supersedes the active profile before Line 57 inserts the replacement, and the supersede result is ignored. An update failure can create two active profiles; an insert failure after superseding can leave no active profile. Wrap supersede+insert in a DB transaction/RPC, or at minimum check the supersede result before proceeding.
🤖 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 `@lib/services/memory-profile-service.ts` around lines 56 - 58, The rollover
logic in memory-profile-service’s profile write path is not atomic because the
existing record is superseded before the replacement insert and the update
result is ignored. Update the flow around the existing
client.from("memory_profiles") update/insert sequence so the supersede result is
checked before continuing, or move both steps into a single transactional
RPC/helper on the database side to ensure only one active profile can exist. If
you keep the current structure, handle any update failure by returning an error
before attempting the insert, and keep the existing insert failure handling in
the same profile write function.
| export const digestSessionSummary=createSessionDigest; | ||
| export async function compoundSessionIntoProfiles(client:MemoryBridgeDbClient,input:{user_id:string;namespace:MemoryBridgeNamespace;since?:string;dry_run?:boolean}){ const r=await runDailyMemoryCompaction(client,input); return {ok:r.ok,data:{updated_profiles:r.updated_profiles,opened_or_updated_loops:r.opened_or_updated_loops},blockers:r.blockers,warnings:r.warnings}; } | ||
| export async function createDailyPackFromSessionDigests(client:MemoryBridgeDbClient,input:{user_id:string;namespace:MemoryBridgeNamespace;since?:string;dry_run?:boolean}){ const r=await runDailyMemoryCompaction(client,input); return {ok:r.ok,data:r.daily_pack,blockers:r.blockers,warnings:r.warnings}; } | ||
| export async function createMasterPackFromProfiles(client:MemoryBridgeDbClient,input:{user_id:string;namespace:MemoryBridgeNamespace;dry_run?:boolean}){ const events=await (client.from("memory_events").select("*").eq("user_id",input.user_id).eq("namespace",input.namespace).order("created_at",{ascending:false}).limit(50) as any as Promise<{data:any[]|null;error:{message:string}|null}>); if(events.error)return {ok:false,data:null,blockers:["event_read_failed"],warnings:[events.error.message]}; const pack=buildMasterContextPack(input.namespace,input.user_id,events.data??[]); if(input.dry_run)return {ok:true,data:pack,blockers:[],warnings:[]}; const ins=await client.from("memory_context_packs").insert({...pack,status:"active"}).select("*").single(); return {ok:!ins.error,data:ins.data??pack,blockers:ins.error?["master_pack_write_failed"]:[],warnings:ins.error?[ins.error.message]:[]}; } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact and filter master-pack source events.
Line 12 builds the master pack from raw memory_events, unlike daily compaction which sanitizes event text first. Filter blocked-secret rows and redact text before calling buildMasterContextPack to avoid persisting sensitive content.
🤖 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 `@lib/services/memory-session-digest-service.ts` at line 12, The master pack is
being built directly from raw memory_events in createMasterPackFromProfiles, so
sensitive source text can be persisted. Update createMasterPackFromProfiles to
filter out blocked-secret rows and redact event text before passing the events
into buildMasterContextPack, matching the sanitization used by the daily
compaction flow.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Supersede existing active master packs before insert.
Line 12 inserts every generated master pack as status: "active" without superseding the previous active master pack. Repeated calls can make active-pack retrieval ambiguous for the same (user_id, namespace, pack_type).
🤖 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 `@lib/services/memory-session-digest-service.ts` at line 12, The
createMasterPackFromProfiles flow is inserting a new active master pack without
retiring the previous one, which can leave multiple active packs for the same
user and namespace. Update createMasterPackFromProfiles to supersede any
existing active master pack for the same (user_id, namespace, pack_type) before
the insert, ideally in the same operation flow as the memory_context_packs
write. Use the createMasterPackFromProfiles function and the
memory_context_packs insert path as the anchors when applying the fix.
| import { runDailyMemoryCompaction } from "../lib/services/memory-compaction-service"; | ||
| import { upsertVersionedMemoryProfile } from "../lib/services/memory-profile-service"; | ||
| import { upsertOpenLoop } from "../lib/services/memory-open-loop-service"; | ||
| function client(seed:Partial<Record<string,any[]>>={}){const rows:Record<string,any[]>={memory_events:[],memory_capture_candidates:[],memory_feedback_events:[],memory_profiles:[],memory_open_loops:[],memory_context_packs:[],...seed}; return {rows,from(table:string){let selected=[...(rows[table]??[])];let patch:any;let insertValue:any;const api:any={select:()=>api,eq:(k:string,v:any)=>{selected=selected.filter(r=>r[k]===v);return api;},order:(k:string,opt:any)=>{selected.sort((a,b)=>opt?.ascending?String(a[k]??"").localeCompare(String(b[k]??"")):String(b[k]??"").localeCompare(String(a[k]??"")));return api;},limit:(n:number)=>{selected=selected.slice(0,n);return api;},update:(v:any)=>{patch=v;return api;},insert:(v:any)=>{insertValue=v;return api;},single:async()=>{if(insertValue){const row={id:`${table}-${rows[table].length+1}`,...insertValue};rows[table].push(row);return {data:row,error:null};} if(patch){for(const r of selected)Object.assign(r,patch);return {data:selected[0]??null,error:selected[0]?null:{message:"not found"}};} return {data:selected[0]??null,error:selected[0]?null:{message:"not found"}};},then:(resolve:any)=>resolve({data:selected,error:null})};return api;}} as any;} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Mock client order sorts versions as strings, causing numeric sort bugs.
The mock client's order comparator uses String(a[k]??"").localeCompare(String(b[k]??"")). When the service queries memory_profiles with .order("version", {ascending:false}), version 10 sorts as "10" and collates before "2", breaking the limit(1) contract that should return the highest numeric version. This can cause false positives or flakiness in tests that rely on correct version ordering (e.g., the supersession logic in upsertVersionedMemoryProfile). Use a numeric comparator when the key is version.
🔧 Proposed fix for version sorting
order:(k:string,opt:any)=>{selected.sort((a,b)=>{
- const cmp=opt?.ascending?String(a[k]??"").localeCompare(String(b[k]??"")):String(b[k]??"").localeCompare(String(a[k]??""));
+ const av=a[k]??"", bv=b[k]??"";
+ const cmp=typeof av==="number"&&typeof bv==="number"
+ ? (opt?.ascending?av-bv:bv-av)
+ : (opt?.ascending?String(av).localeCompare(String(bv)):String(bv).localeCompare(String(av)));
return cmp;
});return api;},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function client(seed:Partial<Record<string,any[]>>={}){const rows:Record<string,any[]>={memory_events:[],memory_capture_candidates:[],memory_feedback_events:[],memory_profiles:[],memory_open_loops:[],memory_context_packs:[],...seed}; return {rows,from(table:string){let selected=[...(rows[table]??[])];let patch:any;let insertValue:any;const api:any={select:()=>api,eq:(k:string,v:any)=>{selected=selected.filter(r=>r[k]===v);return api;},order:(k:string,opt:any)=>{selected.sort((a,b)=>opt?.ascending?String(a[k]??"").localeCompare(String(b[k]??"")):String(b[k]??"").localeCompare(String(a[k]??"")));return api;},limit:(n:number)=>{selected=selected.slice(0,n);return api;},update:(v:any)=>{patch=v;return api;},insert:(v:any)=>{insertValue=v;return api;},single:async()=>{if(insertValue){const row={id:`${table}-${rows[table].length+1}`,...insertValue};rows[table].push(row);return {data:row,error:null};} if(patch){for(const r of selected)Object.assign(r,patch);return {data:selected[0]??null,error:selected[0]?null:{message:"not found"}};} return {data:selected[0]??null,error:selected[0]?null:{message:"not found"}};},then:(resolve:any)=>resolve({data:selected,error:null})};return api;}} as any;} | |
| function client(seed:Partial<Record<string,any[]>>={}){const rows:Record<string,any[]>={memory_events:[],memory_capture_candidates:[],memory_feedback_events:[],memory_profiles:[],memory_open_loops:[],memory_context_packs:[],...seed}; return {rows,from(table:string){let selected=[...(rows[table]??[])];let patch:any;let insertValue:any;const api:any={select:()=>api,eq:(k:string,v:any)=>{selected=selected.filter(r=>r[k]===v);return api;},order:(k:string,opt:any)=>{selected.sort((a,b)=>{const av=a[k]??"", bv=b[k]??"";const cmp=typeof av==="number"&&typeof bv==="number"?(opt?.ascending?av-bv:bv-av):(opt?.ascending?String(av).localeCompare(String(bv)):String(bv).localeCompare(String(av)));return cmp;});return api;},limit:(n:number)=>{selected=selected.slice(0,n);return api;},update:(v:any)=>{patch=v;return api;},insert:(v:any)=>{insertValue=v;return api;},single:async()=>{if(insertValue){const row={id:`${table}-${rows[table].length+1}`,...insertValue};rows[table].push(row);return {data:row,error:null};} if(patch){for(const r of selected)Object.assign(r,patch);return {data:selected[0]??null,error:selected[0]?null:{message:"not found"}};} return {data:selected[0]??null,error:selected[0]?null:{message:"not found"}};},then:(resolve:any)=>resolve({data:selected,error:null})};return api;}} as any;} |
🤖 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 `@tests/phase5c-memory-compaction.test.ts` at line 6, The mock client’s order
helper in client is comparing every field as strings, which breaks numeric
ordering for version and can make limit(1) return the wrong memory_profiles row.
Update the comparator inside client.from(...).order(...) to handle the version
key numerically while preserving the existing string-based fallback for other
columns. Keep the behavior consistent for ascending and descending so tests that
exercise upsertVersionedMemoryProfile and related memory profile queries stay
deterministic.
| const ev=(id:string,raw_text:string,namespace="real_life",over:any={})=>({id,user_id:"u1",namespace,source:"project_update",raw_text,extracted_summary:raw_text,importance:7,sensitivity:"medium",status:"captured",created_at:"2026-06-28T00:00:00.000Z",...over}); | ||
|
|
||
| describe("Phase 5C deterministic compaction",()=>{ | ||
| it("creates project, operating, style, risk, relationship, and daily pack outputs",async()=>{const c=client({memory_events:[ev("e1","Pandora Memory PR pending review in GitHub; Vercel deployment blocked"),ev("e2","I prefer be concise and don't overpraise; use direct tone"),ev("e3","money risk and reputation risk detected"),ev("e4","Patty relationship loop unresolved")],memory_capture_candidates:[{id:"c1",user_id:"u1",namespace:"real_life",status:"approved",summary:"Supabase migration pending",created_at:"2026-06-28T00:00:00.000Z"}],memory_feedback_events:[{id:"f1",user_id:"u1",namespace:"real_life",action:"approve",created_at:"2026-06-28T00:00:00.000Z"}]}); const r=await runDailyMemoryCompaction(c,{user_id:"u1",namespace:"real_life",since:"2026-06-27T00:00:00.000Z"}); expect(r.ok).toBe(true); expect(c.rows.memory_profiles.map((p:any)=>p.profile_type)).toEqual(expect.arrayContaining(["project_profile","operating_profile","style_profile","risk_profile","relationship_loop_profile"])); expect(c.rows.memory_open_loops.length).toBeGreaterThan(0); expect(c.rows.memory_context_packs[0].pack_type).toBe("daily"); expect(JSON.stringify(r.daily_pack)).not.toContain("password"); }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Vacuous redaction assertion: no test data contains "password".
Line 10 asserts expect(JSON.stringify(r.daily_pack)).not.toContain("password"), yet none of the seeded memory_events include that string (or any blocked-secret text). Without a positive test case containing a secret, this assertion only proves the JSON doesn't randomly contain that substring; it does not validate that the clean redaction actually strips blocked secrets. Seed an event with a known blocked secret and assert its absence in the pack.
🤖 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 `@tests/phase5c-memory-compaction.test.ts` at line 10, The redaction check in
runDailyMemoryCompaction is vacuous because the seeded memory_events never
include a blocked secret to remove. Update the test to add a positive
secret-containing event in the client fixture, then assert that the resulting
daily pack from runDailyMemoryCompaction does not include that specific secret
after cleaning, while keeping the existing pack-type and profile assertions.
| describe("Phase 5C job endpoint",()=>{ | ||
| it("requires token and distillation gate",async()=>{vi.resetModules(); process.env.PANDORA_INTERNAL_JOB_TOKEN="secret"; process.env.PANDORA_ENABLE_MEMORY_DISTILLATION="false"; const route=await import("../app/api/memory/jobs/daily-digest/route"); let res=await route.POST(new Request("http://x",{method:"POST",body:"{}"}) as any); expect(res.status).toBe(401); res=await route.POST(new Request("http://x",{method:"POST",headers:{authorization:"Bearer wrong"},body:"{}"}) as any); expect(res.status).toBe(401); res=await route.POST(new Request("http://x",{method:"POST",headers:{authorization:"Bearer secret"},body:JSON.stringify({namespace:"real_life",dry_run:true,user_id:"u1"})}) as any); expect(res.status).toBe(403); }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Endpoint test leaks process.env mutations and omits the success path.
The test mutates process.env.PANDORA_INTERNAL_JOB_TOKEN and process.env.PANDORA_ENABLE_MEMORY_DISTILLATION without restoring them afterwards, risking cross-test pollution. Additionally, it only asserts 401/403 failure responses and never verifies the 200 success path when the token is valid and memoryDistillationEnabled is true. Add beforeEach/afterEach cleanup and a success-case test.
🔧 Proposed fix
describe("Phase 5C job endpoint",()=>{
+ const originalToken=process.env.PANDORA_INTERNAL_JOB_TOKEN;
+ const originalDistillation=process.env.PANDORA_ENABLE_MEMORY_DISTILLATION;
+ beforeEach(()=>{ vi.resetModules(); });
+ afterEach(()=>{
+ process.env.PANDORA_INTERNAL_JOB_TOKEN=originalToken;
+ process.env.PANDORA_ENABLE_MEMORY_DISTILLATION=originalDistillation;
+ });
+
it("requires token and distillation gate",async()=>{
- vi.resetModules();
process.env.PANDORA_INTERNAL_JOB_TOKEN="secret";
process.env.PANDORA_ENABLE_MEMORY_DISTILLATION="false";
...
});
+ it("returns 200 with valid token and enabled distillation",async()=>{
+ process.env.PANDORA_INTERNAL_JOB_TOKEN="secret";
+ process.env.PANDORA_ENABLE_MEMORY_DISTILLATION="true";
+ const route=await import("../app/api/memory/jobs/daily-digest/route");
+ const res=await route.POST(new Request("http://x",{method:"POST",headers:{authorization:"Bearer secret"},body:JSON.stringify({namespace:"real_life",dry_run:true,user_id:"u1"})}) as any);
+ expect(res.status).toBe(200);
+ });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe("Phase 5C job endpoint",()=>{ | |
| it("requires token and distillation gate",async()=>{vi.resetModules(); process.env.PANDORA_INTERNAL_JOB_TOKEN="secret"; process.env.PANDORA_ENABLE_MEMORY_DISTILLATION="false"; const route=await import("../app/api/memory/jobs/daily-digest/route"); let res=await route.POST(new Request("http://x",{method:"POST",body:"{}"}) as any); expect(res.status).toBe(401); res=await route.POST(new Request("http://x",{method:"POST",headers:{authorization:"Bearer wrong"},body:"{}"}) as any); expect(res.status).toBe(401); res=await route.POST(new Request("http://x",{method:"POST",headers:{authorization:"Bearer secret"},body:JSON.stringify({namespace:"real_life",dry_run:true,user_id:"u1"})}) as any); expect(res.status).toBe(403); }); | |
| }); | |
| describe("Phase 5C job endpoint",()=>{ | |
| const originalToken=process.env.PANDORA_INTERNAL_JOB_TOKEN; | |
| const originalDistillation=process.env.PANDORA_ENABLE_MEMORY_DISTILLATION; | |
| beforeEach(()=>{ vi.resetModules(); }); | |
| afterEach(()=>{ | |
| process.env.PANDORA_INTERNAL_JOB_TOKEN=originalToken; | |
| process.env.PANDORA_ENABLE_MEMORY_DISTILLATION=originalDistillation; | |
| }); | |
| it("requires token and distillation gate",async()=>{ | |
| process.env.PANDORA_INTERNAL_JOB_TOKEN="secret"; | |
| process.env.PANDORA_ENABLE_MEMORY_DISTILLATION="false"; | |
| const route=await import("../app/api/memory/jobs/daily-digest/route"); | |
| let res=await route.POST(new Request("http://x",{method:"POST",body:"{}"}) as any); | |
| expect(res.status).toBe(401); | |
| res=await route.POST(new Request("http://x",{method:"POST",headers:{authorization:"Bearer wrong"},body:"{}"}) as any); | |
| expect(res.status).toBe(401); | |
| res=await route.POST(new Request("http://x",{method:"POST",headers:{authorization:"Bearer secret"},body:JSON.stringify({namespace:"real_life",dry_run:true,user_id:"u1"})}) as any); | |
| expect(res.status).toBe(403); | |
| }); | |
| it("returns 200 with valid token and enabled distillation",async()=>{ | |
| process.env.PANDORA_INTERNAL_JOB_TOKEN="secret"; | |
| process.env.PANDORA_ENABLE_MEMORY_DISTILLATION="true"; | |
| const route=await import("../app/api/memory/jobs/daily-digest/route"); | |
| const res=await route.POST(new Request("http://x",{method:"POST",headers:{authorization:"Bearer secret"},body:JSON.stringify({namespace:"real_life",dry_run:true,user_id:"u1"})}) as any); | |
| expect(res.status).toBe(200); | |
| }); | |
| }); |
🤖 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 `@tests/phase5c-memory-compaction.test.ts` around lines 16 - 18, The Phase 5C
job endpoint test is mutating process.env without cleanup and only covers
failure cases. In tests/phase5c-memory-compaction.test.ts, add setup/teardown
around route imports to save and restore PANDORA_INTERNAL_JOB_TOKEN and
PANDORA_ENABLE_MEMORY_DISTILLATION, then keep the current 401/403 assertions but
also add a success-path case for route.POST when the token is valid and memory
distillation is enabled. Use the same POST handler and route import pattern so
the added test exercises the real 200 response path.
1 participant
Motivation
Description
runDailyMemoryCompaction(client, input)that loads recentmemory_events,memory_capture_candidates, andmemory_feedback_events, synthesizes versioned profiles, upserts/merges open loops, and creates adailymemory_context_packsrow while supportingdry_runand ISOsincetimestamps; redacts blocked secrets and always scopes byuser_idandnamespace. (lib/services/memory-compaction-service.ts)upsertVersionedMemoryProfile(...)which finds active profiles byuser_id/namespace/profile_type/subject_key, supersedes previous active rows, merges evidence refs, incrementsversion, and supportsdry_run. (lib/services/memory-profile-service.ts)upsertOpenLoop(...)that finds open/acknowledged loops, merges evidence refs, preserves/increases severity, updatesnext_action, and supportsdry_run. (lib/services/memory-open-loop-service.ts)compoundSessionIntoProfiles,createDailyPackFromSessionDigests, andcreateMasterPackFromProfilescall the new compaction/distillation paths while preserving exports. (lib/services/memory-session-digest-service.ts)POST /api/memory/jobs/daily-digestwhich requiresPANDORA_INTERNAL_JOB_TOKENand the runtime gatePANDORA_ENABLE_MEMORY_DISTILLATION=true, accepts{ namespace, since?, dry_run?, user_id? }, and runs compaction via a bridge admin client. (app/api/memory/jobs/daily-digest/route.ts)app/admin/memory/compaction/page.tsx)docs/pandora-memory-compaction.md, updateddocs/pandora-memory-autopilot.md)tests/phase5c-memory-compaction.test.ts)Key safety constraints enforced by the changes:
user_idandnamespaceandreal_life/auare never mixed.PANDORA_INTERNAL_JOB_TOKENand distillation gate before execution.Files changed (high level):
lib/services/memory-compaction-service.ts(new)lib/services/memory-profile-service.ts(versioned upsert)lib/services/memory-open-loop-service.ts(upsert/update)lib/services/memory-session-digest-service.ts(compounding wrappers updated)app/api/memory/jobs/daily-digest/route.ts(new internal job endpoint)app/admin/memory/compaction/page.tsx(new admin visibility page)docs/pandora-memory-compaction.md(new)docs/pandora-memory-autopilot.md(updated)tests/phase5c-memory-compaction.test.ts(new)Testing
npm run typecheckand it succeeded with no type errors.npm run lint; lint completed and remaining issues are non-blocking/warnings in unrelated files (fixed a minor prefer-const during the rollout).npm run testand all test suites passed (504 tests passed across the repository, including the new Phase 5C tests).npm run buildand the Next.js production build completed successfully (with informational warnings about Edge runtime/Supabase usage but no build failures).All automated checks succeeded in this environment. Recommended next steps: deploy with
PANDORA_INTERNAL_JOB_TOKENconfigured, keep model/embeddings/semantic retrieval/public persistence disabled, enablePANDORA_ENABLE_MEMORY_DISTILLATION=true, run the job indry_run=trueto inspect outputs, then run per-namespace for real runs.Codex Task
Summary by CodeRabbit
New Features
Bug Fixes
Tests