Skip to content

feat(friction): build dated digests and catch up - #1935

Open
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:feat/friction-log-pr05
Open

salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:feat/friction-log-pr05

Conversation

@salmonumbrella

@salmonumbrella salmonumbrella commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

AgentsView now stores daily Friction Log digests and catches up after time offline. Late sessions and delayed reviews update their day within the configured backfill window. Routine catch-up reads stay within that window. A session contributes to only one day, even when hub processes build at the same time. Local days wait for review to finish; PostgreSQL and CockroachDB hubs use the findings pushed by each source.

Review stays off by default. Catch-up covers seven days by default, using local time on SQLite and UTC on PostgreSQL unless a timezone is configured. SQLite builds share the sync and resync lock, including with --no-sync, and release it between days. Usage-only SQLite archives do not build digests.

Digests retain findings and usage totals and survive full resyncs that retain transcript text. Read APIs and CLI commands remain a follow-up.

Builds on merged #1933 and #1934. Refs #1908.

@roborev-ci

roborev-ci Bot commented Sep 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (b508e19)

Verdict: Changes require fixes for 1 finding.

Medium

  • cmd/agentsview/main.go:323: [friction].seat_patterns is validated and documented, but the production startup path never sets sync.EngineConfig.FrictionDims or calls friction.SeatFromPath, so configured seats have no effect. Compile configured seat patterns during startup and pass a FrictionDims callback to the engine that derives the seat from each session's file path and populates the friction dimensions.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 4s | Total: 14m26s

@wesm

wesm commented Oct 16, 2026

Copy link
Copy Markdown
Member

rebasing

@wesm
wesm force-pushed the feat/friction-log-pr05 branch from b508e19 to d5c0013 Compare October 16, 2026 19:31
@wesm wesm changed the title [JOI-5/15] feat(friction): build dated digests and catch up feat(friction): build dated digests and catch up Oct 16, 2026
@roborev-ci

roborev-ci Bot commented Oct 16, 2026

Copy link
Copy Markdown

roborev: Combined Review (d5c0013)

Verdict: Changes require fixes for 6 findings.

High

  • internal/friction/review/job_test.go:32: The nil-exclusive case runs after the preceding invocation has already built yesterday's digest, so CatchUp has no work remaining. Replacing the nil-exclusive branch with return nil would still pass.

    Fix: Run the nil-exclusive case against a fresh seeded store and assert that it creates the expected digest.

  • internal/friction/review/job_test.go:20: Assertions on Name, Interval, Jitter, Cooldown, and RunAtStart repeat constructor configuration literals, including checking Name twice, without exercising scheduling or startup behavior.

    Fix: Remove the configuration-only assertions on lines 20–25 and retain assertions exercising job execution and digest persistence.

  • internal/db/friction_usage_test.go:160: The test checks cost twice: an independent literal expectation is followed by an expectation calculated using the production conversion helper. The second assertion adds no failure detection for the usage adapter.

    Fix: Delete the second cost assertion and the now-unused friction import, keeping the literal expectation.

Medium

  • cmd/agentsview/main.go:551: With --no-sync, engine is nil and digest work runs without serialization, while manual resync uses a separate on-demand engine. If resync swaps the archive between subject selection and finding retrieval, the digest can read stale or empty findings and permanently record those sessions as digested.

    Fix: Serialize digest work with the same engine used by manual sync and resync, including in --no-sync mode.

  • internal/db/friction_digest.go:198: Restarting an existing full archive with archive_content="usage", --no-sync, and friction enabled still schedules digests from retained findings. SaveFrictionDigest writes fresh transcript text despite the active usage-only policy, and UpdateFrictionDigestRender bypasses the same boundary.

    Fix: Apply requireDerivedTextStorage to both digest write methods and disable digest scheduling for usage-only SQLite archives.

  • internal/db/friction_usage.go:105: The usage conversion discards session-level output-token totals when the breakdown is absent or incomplete. The supported aggregate-only case with TotalOutputTokens=700 and an empty breakdown is frozen as OutputTokens=0.

    Fix: Preserve session-level output totals while reconciling available breakdown data, and add behavioral regression coverage for aggregate-only and incomplete-breakdown usage.


Reviewers: 3x codex, codex (security) | Synthesis: codex, 14s | Total: 11m34s

@wesm
wesm force-pushed the feat/friction-log-pr05 branch from d5c0013 to 4139b56 Compare October 16, 2026 21:19
@roborev-ci

roborev-ci Bot commented Oct 16, 2026

Copy link
Copy Markdown

roborev: Combined Review (4139b56)

Verdict: Changes require fixes for 2 findings.

High

  • internal/friction/review/snapshot_test.go:65: TestSnapshotRoundTrip uses maps with at most one entry, so repeated encoding produces identical bytes even without json.Deterministic(true). The assertion cannot detect the regression it claims to prevent.

    Fix: Remove the repeated encoding assertion while retaining round-trip checks, or use multi-key maps with independently expected canonical ordering.

Medium

  • internal/db/friction_digest.go:57: Midnight DST transitions produce incorrect day bounds. In America/Santiago on 2026-09-06, midnight normalizes into the preceding day and AddDate preserves the shifted hour, causing late sessions to disappear permanently from catch-up digests. Both SQLite and PostgreSQL use this helper.

    Fix: Compute the first valid instant of the requested date and following date independently, explicitly handling nonexistent midnights. Add coverage for a midnight DST transition.


Reviewers: 3x codex, codex (security) | Synthesis: codex, 7s | Total: 11m19s

@wesm
wesm force-pushed the feat/friction-log-pr05 branch from 4139b56 to 3e165cb Compare October 17, 2026 01:01
@roborev-ci

roborev-ci Bot commented Oct 17, 2026

Copy link
Copy Markdown

roborev: Combined Review (3e165cb)

Verdict: Changes require fixes for 2 findings.

Medium

  • internal/db/friction_digest.go:391: A nontrashed SQLite session with an empty ended_at can make EarliestSessionDate return an empty string, causing catch-up to treat the archive as empty. A malformed timestamp can instead block every attempt with an error.

    Fix: Treat empty timestamps as NULL, fall back to started_at, and exclude unparseable activity times from earliest-date discovery. Apply consistent fallback semantics to pending-date and subject selection, and test archives containing both invalid and valid timestamps.

  • internal/postgres/friction_digest.go:279: Concurrent PostgreSQL builders can save the same session in digests for different dates if a push changes its activity date between selection and saving. Membership conflicts are silently ignored while both snapshots and pattern contributions commit, leaving persistent double-counting.

    Fix: Reject cross-date membership conflicts atomically within the save transaction, roll back digest and pattern updates, and rebuild from fresh selections. Enforce the invariant in both stores and add a concurrent-build regression test; serializing complete builds across hub processes is another option.


Reviewers: 3x codex, codex (security) | Synthesis: codex, 12s | Total: 9m51s

@wesm
wesm force-pushed the feat/friction-log-pr05 branch from 3e165cb to 7673454 Compare October 17, 2026 01:58
@roborev-ci

roborev-ci Bot commented Oct 17, 2026

Copy link
Copy Markdown

roborev: Combined Review (7673454)

Verdict: Changes require fixes for 3 findings.

High

  • internal/friction/review/dates_test.go:11: TestLocalDateAndAddDays tests thin wrappers around Go date operations. Existing catch-up tests already cover date formatting, timezone selection, and day increments through observable digest behavior; the rollover assertions add no distinct repository-owned coverage.

    Fix: Remove TestLocalDateAndAddDays and the now-unused dates_test.go file; retain the behavioral catch-up tests.

Medium

  • internal/postgres/friction_digest.go:131: Digest subject selection unconditionally reads PostgreSQL's xmin system column, which CockroachDB does not provide. Every digest build therefore fails on the supported CockroachDB backend, while the capability probe still advertises the feature as available.

    Fix: Use a compatible explicit row-version column or backend-specific version expression in both subject selection and save validation.

  • internal/db/friction_digest.go:417; internal/db/friction_digest.go:418: Every hourly catch-up calls EarliestSessionDate before checking existing digests, reading and parsing every active session timestamp even when all digests are current. This makes work proportional to archive history despite the backfill limit and violates the bounded background-work requirement.

    Fix: Check persisted digest state first and determine earliest activity only when needed for initial catch-up. Use the backfill boundary and bounded pending-date query on subsequent passes, and extend the scaling regression to cover unchanged historical sessions and this query.


Reviewers: 3x codex, codex (security) | Synthesis: codex, 10s | Total: 11m26s

A completed day needs a stable digest even when the daemon was offline.
Keep review opt-in and retain daily membership so resumed sessions and
repeat runs do not inflate recurrence counts.

Use the merged finding pipeline and renderer contracts. Freeze native usage
alongside the digest without reintroducing the deferred persona and spend
presentation.
@wesm
wesm force-pushed the feat/friction-log-pr05 branch from 7673454 to e68cead Compare October 17, 2026 13:18
@roborev-ci

roborev-ci Bot commented Oct 17, 2026

Copy link
Copy Markdown

roborev: Combined Review (e68cead)

Verdict: Changes require fixes for 4 findings.

High

  • internal/postgres/friction_digest_pgtest_test.go:216: TestFrictionAvailabilityProbe adds no unique failure detection: existing tests already assert the successful capability probe and schema readiness, making its additional fixtures and PostgreSQL setup redundant.

    Fix: Delete TestFrictionAvailabilityProbe and retain the existing capability and schema assertions in the other tests.

Medium

  • internal/db/friction_digest.go:177: Scheduled builds use Rebuild=true, whose OR condition against left-joined membership prevents SQLite from restricting sessions through the activity index. Even one late session can trigger an archive-wide scan while holding the sync lock, degrading responsiveness.

    Fix: Split selection into indexed queries or UNION ALL branches for unrecorded subjects within the activity range and existing members selected through friction_digest_sessions.date. Apply the equivalent PostgreSQL change and cover the subject-selection query plan.

  • internal/friction/review/runner.go:425: A deferred date can be skipped permanently if its remaining stale sessions become budget-skipped after a later date builds. FrictionPendingDates then omits it while missingStart has advanced beyond it, leaving its digest missing even within the backfill window.

    Fix: Discover missing digest dates throughout the bounded backfill window. Add coverage for a deferred date whose sessions later become skipped.

  • internal/db/friction_usage.go:137: One session's usage lookup failure discards usage for the entire date. The digest and membership can then be saved without usage, and subsequent catch-up passes do not retry the date unless another session arrives.

    Fix: Handle lookup failures per session, retain successful results, and continue other lookups. Propagate cancellation and add coverage proving one failed lookup preserves other sessions' usage.


Reviewers: 3x codex, codex (security) | Synthesis: codex, 13s | Total: 12m24s

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants