spec: F013 Frame Splitting — feature spec, review, impl spec 012.1#102
spec: F013 Frame Splitting — feature spec, review, impl spec 012.1#102
Conversation
…012.1 - F013 feature spec: split_frames tool, split→execute→synthesize pattern - 3-agent review: architecture 7.0, implementation 6.5, research 7.5 (composite 7.0/10) - 012.1 impl spec: 3-phase build plan (~10-13h total) - V1 scope: inline synthesis, no recursive splits, max 5 sub-agents, 120s timeout - Sub-agents always inherit censors, never get split_frames tool
![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: 6fe98a9c93
ℹ️ 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".
| session_id=session_id, | ||
| instruction=spec["task"], | ||
| subtask_type="frame_split", | ||
| frame_type=spec["frame_type"], |
There was a problem hiding this comment.
This constructor uses session_id and instruction, but the current Subtask ORM model (in nous/storage/models.py) defines parent_session_id and task and also requires agent_id; implementing this snippet as-is will fail at object construction and prevent any split_frames subtask from being created.
Useful? React with 👍 / 👎.
| metadata={ | ||
| "context_hints": spec.get("context_hints", []), | ||
| "max_turns": spec.get("max_turns", 5), | ||
| }, |
There was a problem hiding this comment.
metadata_ instead of reserved metadata field
The proposed Subtask(...) payload sets metadata, but the mapped JSON column on Subtask is metadata_ (to avoid SQLAlchemy’s reserved metadata attribute); using metadata here will raise an invalid keyword error and break frame-split task creation.
Useful? React with 👍 / 👎.
| start_time = datetime.utcnow() | ||
| futures = [] | ||
| for task_id in subtask_ids: | ||
| future = await worker_pool.submit(task_id) |
There was a problem hiding this comment.
This flow assumes SubtaskWorkerPool.submit() returns awaitable futures, but the current worker pool implementation (checked nous/handlers/subtask_worker.py) only runs a dequeue loop and exposes no submit method; without documenting the required API change, the sync barrier design here is not implementable and would fail with AttributeError.
Useful? React with 👍 / 👎.
1 participant
F013 Frame Splitting
3 documents added:
•
docs/features/F013-frame-splitting.md— Feature spec•
docs/reviews/F013-3-agent-review.md— 3-agent review (composite: 7.0/10)•
docs/implementation/012.1-frame-splitting.md— Implementation specKey Design Decisions
• New
split_framestool (not extendedspawn_task) — blocking semantics are fundamentally different• Split → Execute → Synthesize pattern
• V1 scope: inline synthesis only, no recursive splits, no auto-detection, no DAG dependencies
• Sub-agents always inherit censors, never get
split_framestool• Max 5 sub-agents, 120s total timeout
Build Plan (012.1)
• Phase 1 (~4-6h): Frame-aware subtask worker —
frame_type+split_idin Subtask model• Phase 2 (~3-4h):
split_framestool + sync barrier, async gather with timeout, partial failure handling• Phase 3 (~2-3h): Result enrichment, cost metrics, status messages during execution
Review Scores
• Architecture: 7.0/10
• Implementation: 6.5/10
• Research: 7.5/10
• Composite: 7.0/10
Risk Flags
• Cost explosion (5 agents × 5 turns = 25 API calls)
• Synthesis quality
• Inter-dependencies need DAG in v2
+984 lines across 3 files