Document what EditAI does, and give it a page that shows it - #21
Open
deonmenezes wants to merge 2 commits into
Open
deonmenezes wants to merge 2 commits into
deonmenezes wants to merge 2 commits into
Conversation
The README described the timeline tools but not the thing people ask about first: what you can say to it, and what happens at each of the three layers the agent drives (your UI, ours, the ffmpeg encode). It now carries the full 16-tool reference with arguments, the connector story including how to attach an MCP server mid-conversation, both sandboxes and what each is for, the environment table, test counts, and a "Known limits" section, because export writes a render description rather than encoding video and the demo does not make that obvious. The landing page in site/ renders the agent server's own sample project and performs a real ripple delete on it: mapTime() applies the same rule project.ts does, so clips straddling a silence get shorter rather than merely shifting, and 24.0s becomes 21.1s. Its palette is lifted from the editor's stylesheet and favicon rather than invented, so the page and the product read as one thing. Two numbers in the README were wrong and are corrected here: the silence ranges in the walkthrough were sketched rather than read off project.ts (they are 3.2-4.1, 9.6-10.4, 16.8-18.0), and .env.example lives in apps/agent, not the repo root.
Contributor
PR Summary by QodoDocument EditAI and add an interactive landing page
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Contributor
Code Review by Qodo
1. Logo import path absent
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+761
to
+764
| spec.clips.forEach(function(c){ | ||
| var el = document.createElement("div"); | ||
| el.className = "clip clip--" + spec.kind; | ||
| el.textContent = c[2] || ""; |
Contributor
There was a problem hiding this comment.
1. build() uses side-effectful foreach 📘 Rule violation ⚙ Maintainability
The new build() implementation uses nested forEach callbacks to create DOM nodes, append them, and mutate clipEls/gapEls; layout() and the reveal code repeat the same side-effect-driven pattern. The checklist requires for...of or an equivalent loop for multi-statement side effects.
Agent Prompt
## Issue description
Replace multi-statement, side-effect-driven `forEach` callbacks with `for...of` loops.
## Issue Context
The callbacks create and append DOM nodes, mutate arrays, update styles, and register observed elements. Keep the simple `timers.forEach(clearTimeout)` call if desired because the checklist explicitly permits one-line simple function calls.
## Fix Focus Areas
- site/index.html[758-790]
- site/index.html[903-913]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+852
to
+854
| at(1250, function(){ | ||
| approveBtn.classList.add("is-hit"); | ||
| at(160, function(){ approveBtn.classList.remove("is-hit"); finish(); }); |
Contributor
There was a problem hiding this comment.
2. Callbacks use anonymous function 📘 Rule violation ⚙ Maintainability
The new script repeatedly passes anonymous function expressions to timers, event listeners, observers, and array methods even though none requires dynamic this, arguments, super, or new.target. These callbacks must use arrow-function syntax under the checklist.
Agent Prompt
## Issue description
Convert safely replaceable anonymous callback function expressions to arrow functions.
## Issue Context
Keep named function declarations and named IIFEs unchanged; update only anonymous expressions passed as callbacks where lexical binding does not alter behavior.
## Fix Focus Areas
- site/index.html[758-790]
- site/index.html[852-868]
- site/index.html[861-868]
- site/index.html[892-913]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| | *"Duck the music under the voiceover"* | Sets clip volume where the voice track is speaking and restores it where it is not. | `get_project` → `set_volume` | | ||
| | *"Grade it warmer and add grain"* | Writes the ffmpeg filter graph, runs it in the media sandbox, then probes the output to confirm it matches the intent. | `probe_media` → `run_ffmpeg` | | ||
| | *"Kill the room tone"* | Denoises the voice track in the sandbox, leaving the original file untouched beside it. | `run_ffmpeg` (`afftdn`) | | ||
| | *"Put their logo in the corner"* | Searches the live web through Bright Data, scrapes the asset, and brings it into the project. | `search_engine` → `scrape_as_markdown` | |
Contributor
There was a problem hiding this comment.
3. Logo import path absent 🐞 Bug ≡ Correctness
The README says Bright Data can scrape a logo and bring it into the project, but scrape_as_markdown returns text and the timeline exposes no tool or model operation to download, register, or add image media. The advertised ask therefore cannot put the discovered asset on the timeline.
Agent Prompt
## Issue description
The documented logo workflow ends at textual web scraping because there is no media-import operation.
## Issue Context
Either describe Bright Data as research-only, or implement downloading, media registration, image clip support, and timeline insertion.
## Fix Focus Areas
- README.md[71-74]
- site/index.html[539-554]
- apps/agent/src/tools.ts[176-209]
- apps/agent/src/project.ts[4-19]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+181
to
+182
| # your own server, attached the same way | ||
| EDITAI_CONNECTORS=exa,bright-data,motion-graphics bun run setup |
Contributor
There was a problem hiding this comment.
4. Custom connectors are skipped 🐞 Bug ≡ Correctness
The new docs say a self-hosted MCP server can be attached by adding its name to EDITAI_CONNECTORS, but setup only accepts exact names returned by TrueForge's catalog and skips every unknown name. A private motion-graphics server therefore does not attach through the shown command.
Agent Prompt
## Issue description
`EDITAI_CONNECTORS` cannot attach arbitrary self-hosted servers as documented.
## Issue Context
Either state that names must exist in the TrueForge MCP catalog, or add a supported name-to-URL configuration path for custom servers.
## Fix Focus Areas
- README.md[164-187]
- site/index.html[579-593]
- apps/agent/scripts/setup.ts[118-160]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+852
to
+855
| at(1250, function(){ | ||
| approveBtn.classList.add("is-hit"); | ||
| at(160, function(){ approveBtn.classList.remove("is-hit"); finish(); }); | ||
| }); |
Contributor
There was a problem hiding this comment.
5. Demo bypasses user approval 🐞 Bug ≡ Correctness
After showing the approval state, ask() schedules finish() unconditionally, so the destructive edit happens without a click; reduced-motion mode bypasses the approval state entirely. This directly contradicts the page's claim that nothing is cut until the user approves.
Agent Prompt
## Issue description
The hero automatically finishes a destructive operation instead of waiting for user approval.
## Issue Context
Keep the deck in its asking state until the approval button is clicked, including in reduced-motion mode.
## Fix Focus Areas
- site/index.html[827-831]
- site/index.html[849-855]
- site/index.html[859-862]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+168
to
+170
| ```bash | ||
| # keyless, the default | ||
| EDITAI_CONNECTORS=exa bun run setup |
Contributor
There was a problem hiding this comment.
6. Connector commands miss directory 🐞 Bug ≡ Correctness
The connector examples invoke bun run setup without entering apps/agent, so running them from the repository root fails because the root package has no setup script. Users following the new section cannot configure the advertised connectors.
Agent Prompt
## Issue description
The connector examples run a package-local script from the repository root.
## Issue Context
Prefix each example with `cd apps/agent &&`, use Bun's workspace targeting, or add a root delegating script.
## Fix Focus Areas
- README.md[168-182]
- package.json[5-10]
- apps/agent/package.json[6-10]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+463
to
+467
| <span class="c"># 2 · the timeline tools</span> | ||
| cd apps/agent && <b>bun run start</b> <span class="c">→ localhost:8941</span> | ||
|
|
||
| <span class="c"># 3 · wire them together (any one key is enough)</span> | ||
| <span class="v">ANTHROPIC_API_KEY</span>=sk-... <b>bun run setup</b> |
Contributor
There was a problem hiding this comment.
7. Landing setup sequence fails 🐞 Bug ≡ Correctness
The landing page changes into apps/agent for step 2 but never restores the repository root: in one shell the final root-only dev:web command fails, while in separate terminals the unqualified step-3 setup command fails from the root. The displayed four-step sequence is not executable as written.
Agent Prompt
## Issue description
The landing-page setup steps rely on incompatible implicit working directories.
## Issue Context
Use subshells or explicit `cd` commands so both package-local `setup` and root-only `dev:web` run from their defining packages.
## Fix Focus Areas
- site/index.html[460-470]
- package.json[5-10]
- apps/agent/package.json[6-10]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| | *"Grade it warmer and add grain"* | Writes the ffmpeg filter graph, runs it in the media sandbox, then probes the output to confirm it matches the intent. | `probe_media` → `run_ffmpeg` | | ||
| | *"Kill the room tone"* | Denoises the voice track in the sandbox, leaving the original file untouched beside it. | `run_ffmpeg` (`afftdn`) | | ||
| | *"Put their logo in the corner"* | Searches the live web through Bright Data, scrapes the asset, and brings it into the project. | `search_engine` → `scrape_as_markdown` | | ||
| | *"Export it at 1080p"* | Renders, after you approve. | `export_project` | |
Contributor
There was a problem hiding this comment.
8. Export claim contradicts implementation 🐞 Bug ≡ Correctness
The ask table says export_project renders a 1080p export, but the implementation writes only a JSON render description, as the new Known Limits section itself later acknowledges. Readers are given contradictory expectations about the result of the advertised command.
Agent Prompt
## Issue description
The capability table calls a JSON-only export a rendered video.
## Issue Context
Align the ask/tool descriptions with the Known Limits section until actual ffmpeg encoding is wired into `export_project`.
## Fix Focus Areas
- README.md[64-74]
- README.md[137-140]
- README.md[403-408]
- apps/agent/src/project.ts[407-423]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+145
to
+146
| The four gated tools are published with MCP's `destructiveHint` annotation, and the agent declares | ||
| `require_approval_for_tools: ["@destructive", "export_project"]`. That turns the annotation into a |
Contributor
There was a problem hiding this comment.
9. Export annotation described incorrectly 🐞 Bug ⚙ Maintainability
The README says all four gated tools carry destructiveHint, but only the three destructive tools do; export_project uses the ordinary write annotation and is gated separately by name. This gives an incorrect approval-policy model to anyone extending or debugging the tool configuration.
Agent Prompt
## Issue description
The approval documentation conflates destructive annotations with explicit name-based gating.
## Issue Context
State that three tools carry `destructiveHint` and `export_project` is separately listed in `require_approval_for_tools`.
## Fix Focus Areas
- README.md[145-148]
- apps/agent/src/tools.ts[48-50]
- apps/agent/src/tools.ts[224-270]
- apps/agent/agent.json[16-19]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+731
to
+735
| var LANES = { | ||
| V1: {kind:"video", clips:[[0,5,"intro.mp4"],[5,11,"b-roll.mp4"],[11,24,"talking-head.mp4"]]}, | ||
| A1: {kind:"audio", clips:[[0,3.2,""],[4.1,9.6,""],[10.4,16.8,""],[18,24,""]]}, | ||
| T1: {kind:"text", clips:[[0.5,4,"Hook line"],[20,24,"Subscribe"]]} | ||
| }; |
Contributor
There was a problem hiding this comment.
10. Sample timeline omits music 🐞 Bug ≡ Correctness
The hero claims to show the server's sample project and a ripple delete across every track, but its lane data omits the seeded A2 music track entirely. The visualization therefore does not show the operation across the complete real sample it claims to reproduce.
Agent Prompt
## Issue description
The landing-page timeline omits one track from the sample project it claims to reproduce.
## Issue Context
Add A2 with its 24-second music clip, or explicitly say the hero shows selected tracks rather than every sample track.
## Fix Focus Areas
- site/index.html[401-405]
- site/index.html[415-419]
- site/index.html[731-735]
- apps/agent/src/project.ts[72-86]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The navbar CTA said "GitHub" in text, which is the one link on the page people scan for by icon rather than by reading. The mark is defined once as an SVG symbol and used in all three places the link appears: the navbar, the outro button, and the footer, so they read as one system instead of three unrelated links. The violet button picks up an inset top highlight and a soft cast of its own colour, which is what separates a button from a coloured rectangle at this size. Below 480px the label is clipped rather than hidden, so the button becomes a square mark and still announces "GitHub" to a screen reader.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
README. It described the tools but not what you can actually say to the thing. It now covers:
project.ts(sourceOffset, ripple delete ordering, caption merging across cuts)ffmpeg-sandboxtools, and whyrun_pythonis gatedexport_projectwrites a render description rather than encoding video, transcripts and tempo come from sample metadata rather than ASR, and motion graphics have no dedicated tool yetLanding page (
site/, live at https://editai-agent.vercel.app). One staticindex.html, no build step and no dependencies. The hero renders the agent server's own sample project and performs a real ripple delete on it:mapTime()applies the same ruleproject.tsdoes, so clips straddling a silence get shorter rather than merely shifting, and 24.0s becomes 21.1s. Colours come fromapps/web/src/styles.cssand the favicon rather than being invented.GitHub About was updated out-of-band: description, homepage, and topics.
Two corrections
Both were wrong in the README before this PR:
project.ts. The real ones are3.2-4.1,9.6-10.4,16.8-18.0, which is the same 2.9s total..env.examplelives atapps/agent/.env.example, not the repo root.site/README.mdrecords the constraint that keeps this honest: if the sample project changes,SILENCESandLANESinindex.htmlhave to change with it, because the page claims those are real numbers.Testing
bun testinapps/agent(17 pass) andbun run testinapps/web(6 pass) are unchanged by this PR; no source files were touched. The page was checked at 1470px and at 390px, wherescrollWidth === clientWidth(no horizontal overflow), and the reduced-motion path renders the finished edit without the typing or tween.