feat(resummarize): re-run the AI pass on edited meeting/memo transcripts - #526
Conversation
|
@rymalia is attempting to deploy a commit to the evil genius laboratory Team on Vercel. A member of the Team first needs to authorize it. |
|
Note on CI: the four MCP jobs (Lockfile ×3, MCP Server) fail because The two agent-docs sync failures were real (the new resummarize error variants weren't in the generated error catalog) and are fixed by the latest push, which regenerates Rust build/test jobs pass locally on the merged tree: 1168 core + 8 integration + 80 CLI tests, clippy |
|
Heads up on the red checks before you spend any time on them: the MCP Lockfile / MCP Server / CI Gate failures are not yours. Main is mid-release for v0.23.0 - the release pin commit references minutes-sdk@0.23.0, which only appears on npm when the release tag publishes it, so every PR's merge ref fails npm ci in that window. Those checks will heal on a re-run once v0.23.0 is out (shortly). On the PR itself: impressive turnaround, and the safety contract reads exactly like the plan we agreed - the declined-findings section with rationale is exactly how I like disagreements documented. As noted on #523 this lands in the release after 0.23.0, so I will do the full review right after the release ships. Two early notes meanwhile: (1) the CLAUDE.md command-count and commands.rs test-struct updates are fine to keep in scope; (2) please rebase on main once 0.23.0 is tagged so the generated site constants regenerate against the released tree - that has bitten two PRs this week. |
…pts (silverstein#523) Adds minutes-core::resummarize + `minutes resummarize <meeting>`: re-runs the summarize stage and local derivations against the current transcript and splices only the AI-owned sections back in. - Hard no-write on engine "none", provider error, empty, scaffolding-only, or unstructured-regression output; concurrent-edit guard compared before the backup copy and again immediately before the atomic rename; fence-aware fail-closed section parser (CommonMark fence-length + closing syntax); file modes enforced via set_permissions on the open handle (umask-proof — preserves 0640 Team files). - Status-preserving merge by exact normalized identity, never fuzzy; every conflict/keep/drop/ambiguity surfaced in the preview (M2). - Timestamped backups (.<name>.pre-resummarize.<unix-secs>.bak), newest 3 kept per artifact; the report carries the actual backup path. - Engine resolution stays inside summarize_with_template (M1); the summarization: health block follows the speaker_mapping shape (M4); preview states the model was invoked (M3); v1 is text-only CLI+core (M5). - CLI: preview by default, --apply/--engine/--template/--json (JSON error envelope on failures); serves `minutes import text` archives as their first AI pass. 1158 core + 8 integration + 80 CLI tests; clippy -D warnings clean.
12efe25 to
7231efc
Compare
silverstein
left a comment
There was a problem hiding this comment.
Full review done: approved. This is the most disciplined feature contribution this repo has received, and the safety contract is not just described - it is implemented and tested.
Verified independently (fresh worktree, own test runs, plus reading every safety-critical path):
Merge semantics (the part I flagged as hardest): correct. Exact normalized identity with uniqueness required on BOTH sides before anything matches - duplicates go to Ambiguous rather than being guessed at, user-stated items survive unmatched, conflicts keep the user's value and emit an explicit note, and every drop is surfaced. That is the wrong-is-worse-than-none doctrine implemented, not just cited.
Splice: fail-closed as specified. All AI-owned ranges resolved strictly before any mutation; duplicate headings abort; covered by tests.
Engine choke point (M1): honored. No engine resolution anywhere in resummarize - everything flows through summarize_with_template, with an injectable seam that makes the failure/concurrency/splice contract testable without a live engine. This composes correctly with the in-flight per-meeting egress policy work.
Write safety: better than the plan asked for. Unique tmp names, mode-correct from the first byte, post-write frontmatter validation, the concurrent-edit guard compared twice (before backup and again before rename), failed runs cleaning up their own backups, newest-3 retention. The residual compare-to-rename window is inherent without file locking and acceptable for a user-edited file.
Both declined findings: agreed with your rationale. Assignee/due are extractor-derived each pass, so treating them as user state would make items undroppable; and the unknown-YAML-keys limitation matches the existing update_frontmatter precedent - an orthogonal project, correctly kept out of scope.
My runs: 34/34 resummarize + 81/81 CLI green, scoped clippy -D warnings clean, fmt clean. Your branch CI already covered the full three-platform matrix; the only reds were our release-window npm ci failures, now healed since v0.23.0 published.
One mechanical step before merge: rebase onto current main (v0.23.0 shipped, so the site constants need regenerating against the released tree - node scripts/generate_llms_txt.mjs && node scripts/sync_site_release_version.mjs, commit site/) and push. Green CI after that and this merges as the headline of the next release.
Merged main (v0.23.0 release) into the resummarize branch and regenerated llms.txt, error catalog, and MCP tool docs so counts reflect the released tree (58 commands, 1660 tests). No resummarize logic touched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
I pushed a mechanical update to your branch (maintainer edit) to unblock the two red checks — no resummarize logic touched:
Once CI is green on the new head I'll squash-merge. This lands as the headline of the next release. Thanks again for an exceptionally clean feature. |
Implements #523 — re-run the AI pass on an edited meeting/memo transcript,
per the maintainer-approved plan (
docs/plans/rerun-ai-pass-resummarize-2026-07-21.md,maintainer notes M1–M5 folded in).
What this adds
minutes-core::resummarize::resummarize_meeting— re-runs thesummarize stage + local derivations (action items, decisions, intents,
entity links) against the current transcript text and splices the
regenerated AI-owned sections back in under a strict safety contract:
none, provider errors, emptyoutput, scaffolding-only output, and unstructured-regression output are
hard no-write failures.
re-compared before the backup copy and again immediately before the
atomic rename.
(
## Summary…## Commitments) and derived frontmatter change;## Notes,## Transcript,speaker_map, capture/consent metadata arepreserved byte-for-byte / field-for-field. Fence-aware, fail-closed
section parser (duplicate headings reject the write).
fuzzy. User-curated state (
status: done, due dates, decisionauthority/supersession) carries forward; everything ambiguous, unmatched,
or dropped is surfaced in the preview, never silently resolved.
.<name>.pre-resummarize.<unix-secs>.bak(mode-preserving); the newest3 per artifact are kept.
minutes resummarize <meeting|memo|search-term>— preview bydefault (the preview does invoke the model, stated in the output per
M3);
--apply,--engine,--template,--json(errors also emit a JSONenvelope). Serves
minutes import textarchives (feat(import): generic importer for existing text transcript archives #516) as their first AIpass.
summarization:health block shaped like the existingspeaker_mappingblock (model, template, duration_ms, last_run).summarize_with_template— resummarize passes config through and can neverbypass per-meeting egress policy. Note for the in-flight privacy work:
this adds a second caller of that choke point.
follow-ups (plan Phases 3–4).
Review findings intentionally declined (with rationale)
Two findings from the Codex review rounds were declined rather than fixed:
Assignee and due are extractor-derived on every fresh pass (
@name:prefixes), so treating assignee as user-curated state would make items
effectively undroppable. Only
status != openand an explicit due datecount as user state — and every drop is surfaced in the preview
regardless.
the in-tree
update_frontmatterprecedent thatredo-speaker-mappingalready ships with; fixing it is an orthogonalschema-preservation project, not part of this feature.
Clarification:
sensitivity: restrictedartifactsRestricted artifacts are not blanket-rejected on purpose: generated
sensitive artifacts carry
capture: none, and that gate already rejects them(their "transcript" may be a placeholder plus human-authored debrief).
A restricted artifact with a real transcript is a legitimate resummarize
target; on cloud engines the preview/apply output states that the model was
invoked, and the M1 choke point is where per-meeting egress policy will be
enforced.
Notes
(CommonMark closing semantics) — a strict improvement over the historical
CLI transcript extractor, which tracked the char only.
set_permissionson the open handle(fchmod), not
OpenOptions::modealone, which is umask-masked — preserves0640 Team-visibility artifacts under restrictive umasks. A
umask-manipulation test was deliberately skipped (process-global, not
parallel-safe); a 0640 mode-preservation test is included.
number.
retained WAV — true audio redaction is
minutes cleanup/retentionterritory.
Tests
1158 core unit + 8 integration + 80 CLI tests pass (
--no-default-features);workspace clippy
-D warningsandcargo fmt --checkclean. New coverageincludes: the fence-aware section parser (incl. fence-length and
closing-syntax cases), splice, the status-preserving merge (densest coverage,
per M2), concurrent-edit guard + backup isolation on conflict, 0640
mode preservation, scaffolding and unstructured-output rejection, timestamped
backup naming + prune-to-3 retention.