forked from jsl/video_editing_poc
Add cinematic edit approval gate
This commit is contained in:
parent
2dc3ec97f4
commit
9536928054
|
|
@ -60,6 +60,8 @@ video-clipping:
|
||||||
source-directory: ./input/editing/source
|
source-directory: ./input/editing/source
|
||||||
poll-interval-ms: 5000
|
poll-interval-ms: 5000
|
||||||
auto-render-when-plan-appears: false
|
auto-render-when-plan-appears: false
|
||||||
|
require-approval-before-render: true
|
||||||
|
approval-file-name: approved.flag
|
||||||
```
|
```
|
||||||
|
|
||||||
## 3. Prepare A Source Project Folder
|
## 3. Prepare A Source Project Folder
|
||||||
|
|
@ -286,6 +288,18 @@ If you set `VIDEO_EDITING_LOCAL_DIRECTOR_AUTO_RENDER=true`, the service renders
|
||||||
output/edit-projects/porsche-session-001/inbox/edit-plan.json
|
output/edit-projects/porsche-session-001/inbox/edit-plan.json
|
||||||
```
|
```
|
||||||
|
|
||||||
|
By default, automatic rendering is still gated by human review. To approve a validated plan for automatic rendering, create:
|
||||||
|
|
||||||
|
```text
|
||||||
|
output/edit-projects/porsche-session-001/inbox/approved.flag
|
||||||
|
```
|
||||||
|
|
||||||
|
On the next inbox scan, the service renders the already imported `edit-plan.json`. To disable the approval gate for local experiments, set:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
VIDEO_EDITING_LOCAL_DIRECTOR_REQUIRE_APPROVAL=false
|
||||||
|
```
|
||||||
|
|
||||||
## 9. Find The Edited Video
|
## 9. Find The Edited Video
|
||||||
|
|
||||||
After rendering completes, the final output is:
|
After rendering completes, the final output is:
|
||||||
|
|
|
||||||
|
|
@ -317,7 +317,8 @@ The plan should be strict JSON so a cheaper model or deterministic renderer can
|
||||||
- [x] Milestone 16 progress: Added FFmpeg-backed local director integration coverage for car, food, family, and generic fixture projects, verifying category artifacts, highlight candidates, and director prompts.
|
- [x] Milestone 16 progress: Added FFmpeg-backed local director integration coverage for car, food, family, and generic fixture projects, verifying category artifacts, highlight candidates, and director prompts.
|
||||||
- [x] Milestone 17: Add benchmark metrics for analysis time, render time, token usage, asset generation cost, and final output size.
|
- [x] Milestone 17: Add benchmark metrics for analysis time, render time, token usage, asset generation cost, and final output size.
|
||||||
- [x] Milestone 17 progress: Added Micrometer benchmark summaries for analysis clip/error counts, render output duration, render output size, AI director prompt token estimate, and asset generation cost baseline.
|
- [x] Milestone 17 progress: Added Micrometer benchmark summaries for analysis clip/error counts, render output duration, render output size, AI director prompt token estimate, and asset generation cost baseline.
|
||||||
- [ ] Milestone 18: Add a human review mode where the user can approve or edit the AI director plan before rendering.
|
- [x] Milestone 18: Add a human review mode where the user can approve or edit the AI director plan before rendering.
|
||||||
|
- [x] Milestone 18 progress: Added a configurable local director approval gate so imported plans wait for `inbox/approved.flag` before automatic rendering, with profile defaults, runbook guidance, and scanner tests.
|
||||||
|
|
||||||
## Testing Strategy
|
## Testing Strategy
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -806,6 +806,10 @@ public class VideoClippingProperties {
|
||||||
|
|
||||||
private boolean autoRenderWhenPlanAppears = false;
|
private boolean autoRenderWhenPlanAppears = false;
|
||||||
|
|
||||||
|
private boolean requireApprovalBeforeRender = true;
|
||||||
|
|
||||||
|
private String approvalFileName = "approved.flag";
|
||||||
|
|
||||||
public boolean isEnabled() {
|
public boolean isEnabled() {
|
||||||
return enabled;
|
return enabled;
|
||||||
}
|
}
|
||||||
|
|
@ -877,6 +881,22 @@ public class VideoClippingProperties {
|
||||||
public void setAutoRenderWhenPlanAppears(boolean autoRenderWhenPlanAppears) {
|
public void setAutoRenderWhenPlanAppears(boolean autoRenderWhenPlanAppears) {
|
||||||
this.autoRenderWhenPlanAppears = autoRenderWhenPlanAppears;
|
this.autoRenderWhenPlanAppears = autoRenderWhenPlanAppears;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public boolean isRequireApprovalBeforeRender() {
|
||||||
|
return requireApprovalBeforeRender;
|
||||||
|
}
|
||||||
|
|
||||||
|
public void setRequireApprovalBeforeRender(boolean requireApprovalBeforeRender) {
|
||||||
|
this.requireApprovalBeforeRender = requireApprovalBeforeRender;
|
||||||
|
}
|
||||||
|
|
||||||
|
public String getApprovalFileName() {
|
||||||
|
return approvalFileName;
|
||||||
|
}
|
||||||
|
|
||||||
|
public void setApprovalFileName(String approvalFileName) {
|
||||||
|
this.approvalFileName = approvalFileName;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -31,6 +31,8 @@ public class EditPlanInboxScanner {
|
||||||
private final Path projectRoot;
|
private final Path projectRoot;
|
||||||
private final String expectedPlanFileName;
|
private final String expectedPlanFileName;
|
||||||
private final boolean autoRender;
|
private final boolean autoRender;
|
||||||
|
private final boolean requireApprovalBeforeRender;
|
||||||
|
private final String approvalFileName;
|
||||||
private final ObjectMapper objectMapper;
|
private final ObjectMapper objectMapper;
|
||||||
private final EditProjectStore store;
|
private final EditProjectStore store;
|
||||||
private final EditProjectService projectService;
|
private final EditProjectService projectService;
|
||||||
|
|
@ -72,6 +74,8 @@ public class EditPlanInboxScanner {
|
||||||
this.projectRoot = Path.of(properties.getEditing().getProjectDirectory());
|
this.projectRoot = Path.of(properties.getEditing().getProjectDirectory());
|
||||||
this.expectedPlanFileName = properties.getEditing().getLocalDirector().getExpectedPlanFileName();
|
this.expectedPlanFileName = properties.getEditing().getLocalDirector().getExpectedPlanFileName();
|
||||||
this.autoRender = properties.getEditing().getLocalDirector().isAutoRenderWhenPlanAppears();
|
this.autoRender = properties.getEditing().getLocalDirector().isAutoRenderWhenPlanAppears();
|
||||||
|
this.requireApprovalBeforeRender = properties.getEditing().getLocalDirector().isRequireApprovalBeforeRender();
|
||||||
|
this.approvalFileName = safeFileName(properties.getEditing().getLocalDirector().getApprovalFileName());
|
||||||
this.objectMapper = objectMapper;
|
this.objectMapper = objectMapper;
|
||||||
this.store = store;
|
this.store = store;
|
||||||
this.projectService = projectService;
|
this.projectService = projectService;
|
||||||
|
|
@ -89,7 +93,13 @@ public class EditPlanInboxScanner {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
findNextPlan().ifPresent(this::importPlan);
|
Optional<Path> nextPlan = findNextPlan();
|
||||||
|
if (nextPlan.isPresent()) {
|
||||||
|
importPlan(nextPlan.get());
|
||||||
|
} else {
|
||||||
|
findNextApprovedProject().ifPresent(project -> renderIfApproved(project.getFileName().toString(),
|
||||||
|
project.resolve("inbox")));
|
||||||
|
}
|
||||||
} finally {
|
} finally {
|
||||||
scanning.set(false);
|
scanning.set(false);
|
||||||
}
|
}
|
||||||
|
|
@ -111,6 +121,22 @@ public class EditPlanInboxScanner {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Optional<Path> findNextApprovedProject() {
|
||||||
|
if (!autoRender || !requireApprovalBeforeRender || !Files.isDirectory(projectRoot)) {
|
||||||
|
return Optional.empty();
|
||||||
|
}
|
||||||
|
try (Stream<Path> projects = Files.list(projectRoot)) {
|
||||||
|
return projects.filter(Files::isDirectory)
|
||||||
|
.sorted(Comparator.comparing(path -> path.getFileName().toString()))
|
||||||
|
.filter(path -> Files.isRegularFile(path.resolve("edit-plan.json")))
|
||||||
|
.filter(path -> Files.isRegularFile(path.resolve("inbox").resolve(approvalFileName)))
|
||||||
|
.filter(path -> !Files.exists(path.resolve("render-manifest.json")))
|
||||||
|
.findFirst();
|
||||||
|
} catch (IOException ex) {
|
||||||
|
throw new IllegalStateException("Unable to scan approved edit projects: " + projectRoot, ex);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private void importPlan(Path inboxPlan) {
|
private void importPlan(Path inboxPlan) {
|
||||||
String projectId = inboxPlan.getParent().getParent().getFileName().toString();
|
String projectId = inboxPlan.getParent().getParent().getFileName().toString();
|
||||||
log.info("event=edit_plan_inbox_detected project_id={} file={}", projectId, inboxPlan);
|
log.info("event=edit_plan_inbox_detected project_id={} file={}", projectId, inboxPlan);
|
||||||
|
|
@ -134,16 +160,25 @@ public class EditPlanInboxScanner {
|
||||||
move(inboxPlan, inboxPlan.resolveSibling(expectedPlanFileName + ".accepted"));
|
move(inboxPlan, inboxPlan.resolveSibling(expectedPlanFileName + ".accepted"));
|
||||||
EditProjectResponse project = projectService.getProject(projectId);
|
EditProjectResponse project = projectService.getProject(projectId);
|
||||||
projectService.updateProject(projectId, EditProjectStatus.PLANNED, Path.of(project.inputDirectory()), null);
|
projectService.updateProject(projectId, EditProjectStatus.PLANNED, Path.of(project.inputDirectory()), null);
|
||||||
log.info("event=edit_plan_imported project_id={} decision_count={} auto_render={}",
|
log.info("event=edit_plan_imported project_id={} decision_count={} auto_render={} approval_required={}",
|
||||||
projectId, plan.decisions().size(), autoRender);
|
projectId, plan.decisions().size(), autoRender, requireApprovalBeforeRender);
|
||||||
log.info("event=edit_plan_saved project_id={} decision_count={} source=inbox",
|
log.info("event=edit_plan_saved project_id={} decision_count={} source=inbox",
|
||||||
projectId, plan.decisions().size());
|
projectId, plan.decisions().size());
|
||||||
if (autoRender) {
|
if (autoRender) {
|
||||||
renderer.orElseThrow(() -> new IllegalStateException(
|
renderIfApproved(projectId, store.inboxDirectory(projectId));
|
||||||
"Automatic rendering is enabled but no EditRenderer is available")).render(projectId);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void renderIfApproved(String projectId, Path inbox) {
|
||||||
|
if (requireApprovalBeforeRender && !Files.isRegularFile(inbox.resolve(approvalFileName))) {
|
||||||
|
log.info("event=edit_render_waiting_for_approval project_id={} approval_file={}",
|
||||||
|
projectId, inbox.resolve(approvalFileName));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
renderer.orElseThrow(() -> new IllegalStateException(
|
||||||
|
"Automatic rendering is enabled but no EditRenderer is available")).render(projectId);
|
||||||
|
}
|
||||||
|
|
||||||
private EditPlan normalized(EditPlan plan) {
|
private EditPlan normalized(EditPlan plan) {
|
||||||
return new EditPlan(plan.projectId(), plan.style(), plan.targetDurationSeconds(),
|
return new EditPlan(plan.projectId(), plan.style(), plan.targetDurationSeconds(),
|
||||||
List.copyOf(plan.decisions()), List.copyOf(plan.audioCues()), List.copyOf(plan.voiceover()),
|
List.copyOf(plan.decisions()), List.copyOf(plan.audioCues()), List.copyOf(plan.voiceover()),
|
||||||
|
|
@ -157,4 +192,12 @@ public class EditPlanInboxScanner {
|
||||||
throw new IllegalStateException("Unable to archive edit plan inbox file: " + source, ex);
|
throw new IllegalStateException("Unable to archive edit plan inbox file: " + source, ex);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static String safeFileName(String fileName) {
|
||||||
|
if (fileName == null || fileName.isBlank() || !Path.of(fileName).getFileName().toString().equals(fileName)
|
||||||
|
|| fileName.contains("..")) {
|
||||||
|
throw new IllegalArgumentException("Approval file name must be a plain file name: " + fileName);
|
||||||
|
}
|
||||||
|
return fileName;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,8 @@ video-clipping:
|
||||||
local-director:
|
local-director:
|
||||||
enabled: true
|
enabled: true
|
||||||
auto-render-when-plan-appears: false
|
auto-render-when-plan-appears: false
|
||||||
|
require-approval-before-render: true
|
||||||
|
approval-file-name: approved.flag
|
||||||
|
|
||||||
logging:
|
logging:
|
||||||
level:
|
level:
|
||||||
|
|
|
||||||
|
|
@ -53,6 +53,8 @@ video-clipping:
|
||||||
director-prompt-file-name: ${VIDEO_EDITING_LOCAL_DIRECTOR_PROMPT_FILE_NAME:ai-director-prompt.md}
|
director-prompt-file-name: ${VIDEO_EDITING_LOCAL_DIRECTOR_PROMPT_FILE_NAME:ai-director-prompt.md}
|
||||||
expected-plan-file-name: ${VIDEO_EDITING_LOCAL_DIRECTOR_EXPECTED_PLAN_FILE_NAME:edit-plan.json}
|
expected-plan-file-name: ${VIDEO_EDITING_LOCAL_DIRECTOR_EXPECTED_PLAN_FILE_NAME:edit-plan.json}
|
||||||
auto-render-when-plan-appears: ${VIDEO_EDITING_LOCAL_DIRECTOR_AUTO_RENDER:false}
|
auto-render-when-plan-appears: ${VIDEO_EDITING_LOCAL_DIRECTOR_AUTO_RENDER:false}
|
||||||
|
require-approval-before-render: ${VIDEO_EDITING_LOCAL_DIRECTOR_REQUIRE_APPROVAL:true}
|
||||||
|
approval-file-name: ${VIDEO_EDITING_LOCAL_DIRECTOR_APPROVAL_FILE_NAME:approved.flag}
|
||||||
|
|
||||||
logging:
|
logging:
|
||||||
level:
|
level:
|
||||||
|
|
|
||||||
|
|
@ -53,6 +53,8 @@ class VideoClippingPropertiesTest {
|
||||||
assertThat(properties.getEditing().getLocalDirector().getSourceDirectory())
|
assertThat(properties.getEditing().getLocalDirector().getSourceDirectory())
|
||||||
.isEqualTo("./input/editing/source");
|
.isEqualTo("./input/editing/source");
|
||||||
assertThat(properties.getEditing().getLocalDirector().isAutoRenderWhenPlanAppears()).isFalse();
|
assertThat(properties.getEditing().getLocalDirector().isAutoRenderWhenPlanAppears()).isFalse();
|
||||||
|
assertThat(properties.getEditing().getLocalDirector().isRequireApprovalBeforeRender()).isTrue();
|
||||||
|
assertThat(properties.getEditing().getLocalDirector().getApprovalFileName()).isEqualTo("approved.flag");
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -79,7 +81,9 @@ class VideoClippingPropertiesTest {
|
||||||
"video-clipping.editing.local-director.enabled=false",
|
"video-clipping.editing.local-director.enabled=false",
|
||||||
"video-clipping.editing.local-director.source-directory=/tmp/source",
|
"video-clipping.editing.local-director.source-directory=/tmp/source",
|
||||||
"video-clipping.editing.local-director.poll-interval-ms=9000",
|
"video-clipping.editing.local-director.poll-interval-ms=9000",
|
||||||
"video-clipping.editing.local-director.auto-render-when-plan-appears=true"
|
"video-clipping.editing.local-director.auto-render-when-plan-appears=true",
|
||||||
|
"video-clipping.editing.local-director.require-approval-before-render=false",
|
||||||
|
"video-clipping.editing.local-director.approval-file-name=go.flag"
|
||||||
)
|
)
|
||||||
.run(context -> {
|
.run(context -> {
|
||||||
VideoClippingProperties properties = context.getBean(VideoClippingProperties.class);
|
VideoClippingProperties properties = context.getBean(VideoClippingProperties.class);
|
||||||
|
|
@ -104,6 +108,8 @@ class VideoClippingPropertiesTest {
|
||||||
assertThat(properties.getEditing().getLocalDirector().getSourceDirectory()).isEqualTo("/tmp/source");
|
assertThat(properties.getEditing().getLocalDirector().getSourceDirectory()).isEqualTo("/tmp/source");
|
||||||
assertThat(properties.getEditing().getLocalDirector().getPollIntervalMs()).isEqualTo(9000);
|
assertThat(properties.getEditing().getLocalDirector().getPollIntervalMs()).isEqualTo(9000);
|
||||||
assertThat(properties.getEditing().getLocalDirector().isAutoRenderWhenPlanAppears()).isTrue();
|
assertThat(properties.getEditing().getLocalDirector().isAutoRenderWhenPlanAppears()).isTrue();
|
||||||
|
assertThat(properties.getEditing().getLocalDirector().isRequireApprovalBeforeRender()).isFalse();
|
||||||
|
assertThat(properties.getEditing().getLocalDirector().getApprovalFileName()).isEqualTo("go.flag");
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -22,5 +22,9 @@ class CinematicEditingLocalProfileTest {
|
||||||
assertThat(properties.getProperty("video-clipping.editing.local-director.enabled")).isEqualTo("true");
|
assertThat(properties.getProperty("video-clipping.editing.local-director.enabled")).isEqualTo("true");
|
||||||
assertThat(properties.getProperty("video-clipping.editing.local-director.auto-render-when-plan-appears"))
|
assertThat(properties.getProperty("video-clipping.editing.local-director.auto-render-when-plan-appears"))
|
||||||
.isEqualTo("false");
|
.isEqualTo("false");
|
||||||
|
assertThat(properties.getProperty("video-clipping.editing.local-director.require-approval-before-render"))
|
||||||
|
.isEqualTo("true");
|
||||||
|
assertThat(properties.getProperty("video-clipping.editing.local-director.approval-file-name"))
|
||||||
|
.isEqualTo("approved.flag");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -89,12 +89,46 @@ class EditPlanInboxScannerTest {
|
||||||
void invokesRendererWhenAutoRenderIsEnabled() throws Exception {
|
void invokesRendererWhenAutoRenderIsEnabled() throws Exception {
|
||||||
writeValidPlan();
|
writeValidPlan();
|
||||||
EditRenderer renderer = mock(EditRenderer.class);
|
EditRenderer renderer = mock(EditRenderer.class);
|
||||||
|
properties.getEditing().getLocalDirector().setRequireApprovalBeforeRender(false);
|
||||||
|
|
||||||
scanner(true, renderer).scan();
|
scanner(true, renderer).scan();
|
||||||
|
|
||||||
verify(renderer).render("porsche-edit");
|
verify(renderer).render("porsche-edit");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void waitsForApprovalBeforeAutoRendering() throws Exception {
|
||||||
|
writeValidPlan();
|
||||||
|
EditRenderer renderer = mock(EditRenderer.class);
|
||||||
|
|
||||||
|
scanner(true, renderer).scan();
|
||||||
|
|
||||||
|
assertThat(store.editPlanFile("porsche-edit")).exists();
|
||||||
|
assertThat(store.inboxDirectory("porsche-edit").resolve("edit-plan.json.accepted")).exists();
|
||||||
|
verify(renderer, never()).render("porsche-edit");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void rendersApprovedPlanOnLaterScan() throws Exception {
|
||||||
|
writeValidPlan();
|
||||||
|
EditRenderer renderer = mock(EditRenderer.class);
|
||||||
|
EditPlanInboxScanner scanner = scanner(true, renderer);
|
||||||
|
|
||||||
|
scanner.scan();
|
||||||
|
Files.writeString(store.inboxDirectory("porsche-edit").resolve("approved.flag"), "approved");
|
||||||
|
scanner.scan();
|
||||||
|
|
||||||
|
verify(renderer).render("porsche-edit");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void rejectsUnsafeApprovalFileName() {
|
||||||
|
properties.getEditing().getLocalDirector().setApprovalFileName("../approved.flag");
|
||||||
|
|
||||||
|
org.assertj.core.api.Assertions.assertThatIllegalArgumentException().isThrownBy(() ->
|
||||||
|
scanner(true, mock(EditRenderer.class)));
|
||||||
|
}
|
||||||
|
|
||||||
private void writeValidPlan() throws Exception {
|
private void writeValidPlan() throws Exception {
|
||||||
EditPlan plan = new EditPlan("porsche-edit", "cinematic-porsche-promo", 60,
|
EditPlan plan = new EditPlan("porsche-edit", "cinematic-porsche-promo", 60,
|
||||||
List.of(new EditDecision("clip_00001", 0, 2.5, 0, 2.5,
|
List.of(new EditDecision("clip_00001", 0, 2.5, 0, 2.5,
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue