feat(prd): Add comprehensive PRD management commands and versioning

[frankbria]

Summary

• Implements complete PRD management system following TDD principles
• Adds core functions for PRD deletion, export, and versioning
• Adds CLI commands for managing PRDs with version history support
• Includes database schema migration for versioning columns

Changes

Core PRD Functions (codeframe/core/prd.py)

Function	Purpose
delete(workspace, prd_id)	Remove a PRD from workspace
export_to_file(workspace, prd_id, path, force)	Export PRD to file
create_new_version(workspace, prd_id, content, summary)	Create new version
get_versions(workspace, prd_id)	List all versions of a PRD
get_version(workspace, prd_id, version_number)	Get specific version
diff_versions(workspace, prd_id, v1, v2)	Generate unified diff

CLI Commands (codeframe prd)

Command	Description
prd list	List all PRDs with truncated IDs and timestamps
prd show [id]	Show PRD (optionally by specific ID)
prd delete <id> [--force]	Delete PRD with confirmation
prd export <id|latest> <file> [--force]	Export to file
prd versions <id>	Show version history
prd diff <id> <v1> <v2>	Show diff between versions
prd update <id> <file> -m <msg>	Create new version from file

Database Schema

Added versioning columns to prds table:

• version (INTEGER DEFAULT 1) - Version number
• parent_id (TEXT) - Links to previous version
• change_summary (TEXT) - Change description

Includes automatic migration for existing databases.

Test plan

• 38 core PRD tests pass (tests/core/test_prd.py)
• 30 CLI command tests pass (tests/cli/test_prd_commands.py)
• Full v2 test suite passes (635 tests)
• Ruff lint checks pass
• Manual testing of CLI commands
• Verify database migration on existing workspace

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

• develop

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/prd-command-enhancements

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[macroscopeapp]

Add PRD CLI commands for list, delete, export, versions, diff, and update and implement PRD versioning with chain_id in codeframe/cli/app.py, codeframe/core/prd.py, and codeframe/core/workspace.py

Introduce PRD versioning (version, parent_id, change_summary, chain_id) with schema upgrades and indexes, add CLI handlers for listing, deleting, exporting, viewing versions, diffing versions, and updating PRDs, and emit EventType.PRD_DELETED and PRD_UPDATED events. Key changes start in app.py and core logic in prd.py with workspace migrations in workspace.py.

📍Where to Start

Start with PRD command handlers in app.py, then review versioning data model and operations in prd.py, followed by schema upgrades in workspace.py.

---

Macroscope summarized 8109343.

[macroscopeapp]

Migration for prds isn’t resilient: new columns aren’t added and ALTER TABLE assumes the table exists. Suggest checking for the table and missing columns (e.g., sqlite_master, PRAGMA table_info) and only ALTER when needed, or document why prds is guaranteed to exist.

Suggested change

	# Migration: Add new columns to existing prds table
	cursor.execute("PRAGMA table_info(prds)")
	prds_columns = {row[1] for row in cursor.fetchall()}
	if "version" not in prds_columns:
	cursor.execute("ALTER TABLE prds ADD COLUMN version INTEGER DEFAULT 1")
	if "parent_id" not in prds_columns:
	cursor.execute("ALTER TABLE prds ADD COLUMN parent_id TEXT")
	if "change_summary" not in prds_columns:
	cursor.execute("ALTER TABLE prds ADD COLUMN change_summary TEXT")

🚀 Want me to fix this? Reply ex: "fix it for me".

[macroscopeapp]

DB connections can leak on exceptions. Suggest always closing via try/finally or with sqlite3.connect(...) as conn:.

🚀 Want me to fix this? Reply ex: "fix it for me".

[github-actions]

Review Summary

This PR adds comprehensive PRD versioning capabilities including deletion, export, version history, and diff functionality. The implementation follows the v2 headless architecture pattern and includes excellent test coverage (68 new tests).

✅ Strengths

1. Test Coverage: 38 core tests + 30 CLI tests - comprehensive coverage of new functionality
2. Clean Code: Well-structured, readable code with good docstrings
3. Event System: Properly emits PRD_DELETED event for tracking
4. Schema Migration: Handles adding versioning columns to existing databases
5. Headless Design: No FastAPI/HTTP dependencies, follows core-first pattern
6. Error Handling: Good use of exceptions and graceful None returns
7. Idempotent Operations: Database operations can be safely retried

⚠️ Issues to Address

1. Performance: Missing Database Index

Location: codeframe/core/prd.py:424-509

The get_versions() function queries by parent_id to find children, but there's no index on this column. For workspaces with many PRD versions, this could be slow.

Recommendation: Add index in workspace.py:

cursor.execute("CREATE INDEX IF NOT EXISTS idx_prds_parent ON prds(parent_id)")

2. Architecture: No chain_id for Version Grouping

Location: codeframe/core/prd.py:360-421

The versioning system uses parent_id to chain versions, but there's no way to identify that multiple PRD records are versions of the "same" PRD. This makes it difficult to list all PRD chains in a workspace.

Recommendation: Add a chain_id field that stays constant across versions:

• store() sets chain_id = prd_id
• create_new_version() copies parent.chain_id
• Add query: list_chains(workspace) to get all PRD chains

3. Concurrency: Version Number Not Atomic

Location: codeframe/core/prd.py:384

new_version = parent.version + 1  # Read
# ... later ...
(new_version, ...)  # Write

If two create_new_version calls run concurrently, they could get the same version number. While SQLite's transaction isolation helps, explicit transactions would be clearer.

Recommendation: Wrap the insert in an explicit transaction:

with conn:
    cursor.execute(...)
    conn.commit()

4. Multiple Queries for Version Chain

Location: codeframe/core/prd.py:448-503

The get_versions() function makes N+1 database queries (one per version in the chain). For long chains, this is inefficient.

Recommendation: Consider adding a chain_id field and using a single query:

SELECT * FROM prds WHERE workspace_id = ? AND chain_id = ? ORDER BY version DESC

5. Documentation: Commands Not in CLI_WIREFRAME.md

The following new commands are not documented in docs/CLI_WIREFRAME.md, which is the authoritative command mapping:

• prd list
• prd show [id]
• prd delete
• prd export
• prd versions
• prd diff
• prd update

Recommendation: Update CLI_WIREFRAME.md to document these commands and their core function mappings.

6. Edge Case: diff_versions Inconsistent Return

Location: codeframe/core/prd.py:576-571

When versions are identical, difflib.unified_diff() returns an empty iterator, and the function returns an empty string. However, when versions don't exist, it returns None. CLI code at app.py:980-982 checks for None but empty strings just print "No differences" which is actually correct behavior.

This is actually working as intended, but consider making the API more explicit:

• Return None when either version doesn't exist
• Return empty string when versions are identical (current behavior is fine)

7. Missing Validation on Delete

Location: codeframe/core/prd.py:289-313

def delete(workspace: Workspace, prd_id: str) -> bool:
    # ... deletes PRD ...

Doesn't check if tasks depend on this PRD. Deleting a PRD that has generated tasks leaves orphaned task records.

Recommendation: Add a check:

# Check for dependent tasks
cursor.execute("SELECT COUNT(*) FROM tasks WHERE prd_id = ?", (prd_id,))
task_count = cursor.fetchone()[0]
if task_count > 0:
    raise ValueError(f"Cannot delete PRD with {task_count} dependent tasks")

📋 Code Quality Observations

• ✅ No SQL injection (parameterized queries used everywhere)
• ✅ Type hints present on all functions
• ✅ Proper exception handling (FileNotFoundError, FileExistsError, etc.)
• ✅ Good use of dataclasses for domain models
• ✅ Follows repository conventions (no comments, headless design)

🧪 Testing

The test coverage is excellent with 68 new tests. However, consider adding:

1. Concurrent version creation tests: Verify version number uniqueness
2. Performance tests: Test with large version chains (100+ versions)
3. Migration tests: Verify _ensure_schema_upgrades handles edge cases

🔒 Security

No security concerns identified:

• No user input directly in SQL queries
• File operations use Path (safe)
• No secrets or credentials handled
• Follows principle of least privilege

---

Summary

This is a high-quality implementation of PRD versioning features that adds significant functionality to CodeFRAME. The code is well-tested, clean, and follows the v2 architecture principles.

Recommended actions before merge:

1. Add index on prds.parent_id for performance
2. Update docs/CLI_WIREFRAME.md to document new commands
3. Consider adding chain_id field for better version grouping (can be a follow-up)
4. Add validation to prevent deleting PRDs with dependent tasks

Nice to have (follow-up):

• Optimize get_versions() to use single query with chain_id
• Add explicit transactions for version creation
• Add performance tests for large version chains

Overall, this PR is ready to merge with minor improvements for performance and documentation completeness.

[github-actions]

Posted comprehensive review comment to the PR. The review covers code quality, performance considerations (missing indexes on parent_id), architecture concerns (no chain_id for version grouping), concurrency issues, documentation gaps in CLI_WIREFRAME.md, edge cases, and validation gaps. The PR is well-implemented with excellent test coverage and follows v2 architecture principles, but would benefit from the recommended performance optimizations and documentation updates before merging.

opencode session  |  github run

[github-actions]

Remaining Review Items

Following up on the comprehensive reviews already posted, here are the key issues still needing attention:

🚨 Must Fix Before Merge

1. Missing Index on parent_id (Performance)
Location: codeframe/core/workspace.py

The get_versions() function queries by parent_id to find children. Without an index, this degrades linearly with the number of PRD versions.

# Add after line 96 in _ensure_schema_upgrades():
cursor.execute("CREATE INDEX IF NOT EXISTS idx_prds_parent ON prds(parent_id)")

2. Missing Cycle Protection in get_versions()
Location: codeframe/core/prd.py:446-456

The "walk up" logic to find the root version has no cycle detection. While schema invariants should prevent cycles, defensive code is better:

# Replace lines 446-456 with:
current_id = prd_id
visited_up = set()
max_depth = 1000  # Safety limit

while len(visited_up) < max_depth:
    cursor.execute(
        "SELECT parent_id FROM prds WHERE workspace_id = ? AND id = ?",
        (workspace.id, current_id),
    )
    row = cursor.fetchone()
    if not row or not row[0]:
        break
    if row[0] in visited_up:
        # Cycle detected - treat current as root
        break
    visited_up.add(row[0])
    current_id = row[0]

📋 Documentation Gap

3. Missing CLI Documentation
Location: docs/CLI_WIREFRAME.md

Per AGENTS.md, this file is the authoritative command mapping. Add entries for:

• prd list → list_prds()
• prd show [id] → get_by_id() / get_latest()
• prd delete <id> → delete()
• prd export <id|latest> <file> → export_to_file()
• prd versions <id> → get_versions()
• prd diff <id> <v1> <v2> → diff_versions()
• prd update <id> <file> -m <msg> → create_new_version()

---

Summary: The core implementation is solid with excellent test coverage. Addressing the index and cycle protection will improve robustness and performance. Documentation update ensures the command mapping stays authoritative.

[macroscopeapp]

Diff lines may contain [/], which Rich treats as markup, causing wrong rendering or errors. Consider disabling markup on these prints and use style= for color.

Suggested change

	for line in diff.splitlines():
	if line.startswith("+") and not line.startswith("+++"):
	console.print(f"[green]{line}[/green]")
	elif line.startswith("-") and not line.startswith("---"):
	console.print(f"[red]{line}[/red]")
	elif line.startswith("@@"):
	console.print(f"[cyan]{line}[/cyan]")
	else:
	console.print(line)
	for line in diff.splitlines():
	if line.startswith("+") and not line.startswith("+++"):
	console.print(line, style="green", markup=False)
	elif line.startswith("-") and not line.startswith("---"):
	console.print(line, style="red", markup=False)
	elif line.startswith("@@"):
	console.print(line, style="cyan", markup=False)
	else:
	console.print(line, markup=False)

🚀 Want me to fix this? Reply ex: "fix it for me".

[github-actions]

Review posted to PR #293.

opencode session  |  github run

[github-actions]

Follow-up Review: Most Issues Addressed ✅

I reviewed the current state of the code and found that most issues from previous reviews have been addressed. The implementation has evolved significantly since the initial reviews.

✅ Issues Fixed in Current Code

1. Missing index on parent_id - FIXED

   • Added at and also added index on chain_id at line 280

2. No chain_id for version grouping - FIXED

   • chain_id column fully implemented and used throughout
   • New function added for listing PRD chains

3. Concurrency: Version number not atomic - FIXED

   • now uses explicit transaction with (prd.py:476)

4. Multiple queries for version chain - FIXED

   • now uses single query with chain_id (prd.py:581-590)

5. Missing validation on delete - FIXED

   • Added exception
   • function has parameter

6. Migration resilience - FIXED

   • Migration code properly uses before each ALTER (workspace.py:257-276)
   • Includes backfill for chain_id for existing PRDs

7. lineterm diff issue - FIXED

   • Uses instead of empty string (prd.py:667)

8. DB connection leaks - FIXED

   • uses (prd.py:545)

⚠️ One Remaining Issue

Diff Lines May Contain Markup Characters
Location:

Diff lines can contain and characters (e.g., in code snippets), which Rich treats as markup. This can cause rendering issues or errors.

Current code:

Recommended fix:

This ensures diff output is rendered correctly regardless of content.

📝 Documentation

The was updated in this PR (+118/-3 lines), which addresses the documentation gap noted in earlier reviews.

---

Summary

This is excellent work! The implementation has matured well, addressing all major concerns from earlier reviews. The code now has:

• Proper transaction handling for version creation
• Efficient single-query version lookup using chain_id
• Comprehensive dependency checking for deletions
• Resilient database migrations with backfill
• Proper connection management

Only one minor issue remains with diff output rendering. Otherwise, this PR is in excellent shape for merging.

[github-actions]

Review posted successfully. Found that most issues from earlier reviews have been addressed, with only one minor rendering issue remaining in the diff output.

opencode session  |  github run

[macroscopeapp]

NULL chain_id is dropped by equality joins/filters. Consider a canonical key (e.g., COALESCE(chain_id, id)) used consistently, or branch with IS NULL.

Suggested change

	INNER JOIN (
	SELECT chain_id, MAX(version) as max_version
	FROM prds
	WHERE workspace_id = ?
	GROUP BY chain_id
	) latest ON p.chain_id = latest.chain_id AND p.version = latest.max_version
	INNER JOIN (
	SELECT COALESCE(chain_id, id) AS chain_key, MAX(version) as max_version
	FROM prds
	WHERE workspace_id = ?
	GROUP BY chain_key
	) latest ON COALESCE(p.chain_id, p.id) = latest.chain_key AND p.version = latest.max_version

🚀 Want me to fix this? Reply ex: "fix it for me".

[macroscopeapp]

ROLLBACK in the except block can fail and mask the original error (e.g., if the connection is already closed). Consider wrapping ROLLBACK in its own try/except so the original exception is preserved.

Suggested change

	except Exception:
	cursor.execute("ROLLBACK")
	raise
	except Exception:
	try:
	cursor.execute("ROLLBACK")
	except Exception:
	pass
	raise

🚀 Want me to fix this? Reply ex: "fix it for me".

[github-actions]

Final Review: Critical Markup Issue Found ⚠️

This PR is well-implemented with excellent test coverage (81 tests). Most issues from prior reviews have been addressed. However, one critical issue remains that could affect user experience:

🐛 Critical: Diff Output Markup Injection

Location:

Diff lines can contain and characters (e.g., in code snippets or markdown), which Rich interprets as markup tags. This causes rendering errors or incorrect formatting.

Current code:

Fix: Add parameter to prevent markup interpretation:

✅ Confirmed Fixed Issues

• Database indexes added on and
• Version creation uses explicit transactions
• Single-query version lookup with
• Task dependency validation on delete
• Delete result checked in CLI
• Documentation updated in
• Resilient schema migration with backfill

📝 Minor Defensive Issues (Low Priority)

1. ROLLBACK error masking (): If ROLLBACK fails, it could mask the original exception. Wrap in its own try/except for defense in depth.

2. NULL chain_id filtering (): INNER JOIN filters records with NULL chain_id. This is defensive since migration backfill and both set , but could use for extra safety.

---

Summary: Excellent implementation with one user-facing bug that should be fixed. The markup injection issue can cause confusing output when diffing PRDs containing code or markdown.

[github-actions]

Final Review: Critical Markup Issue Found ⚠️

This PR is well-implemented with excellent test coverage (81 tests). Most issues from prior reviews have been addressed. However, one critical issue remains that could affect user experience:

🐛 Critical: Diff Output Markup Injection

Location: codeframe/cli/app.py:990-997

Diff lines can contain [ and ] characters (e.g., in code snippets or markdown), which Rich interprets as markup tags. This causes rendering errors or incorrect formatting.

Fix: Add markup=False parameter to prevent markup interpretation:

• Line 991: console.print(line, style="green", markup=False)
• Line 993: console.print(line, style="red", markup=False)
• Line 995: console.print(line, style="cyan", markup=False)
• Line 997: console.print(line, markup=False)

✅ Confirmed Fixed Issues

• Database indexes added on parent_id and chain_id
• Version creation uses explicit transactions
• Single-query version lookup with chain_id
• Task dependency validation on delete
• Delete result checked in CLI
• Documentation updated in CLI_WIREFRAME.md
• Resilient schema migration with backfill

📝 Minor Defensive Issues (Low Priority)

1. ROLLBACK error masking (prd.py:542): If ROLLBACK fails, it could mask the original exception. Wrap in its own try/except for defense in depth.

2. NULL chain_id filtering (prd.py:331-336): INNER JOIN filters records with NULL chain_id. This is defensive since migration backfill and store() both set chain_id, but could use COALESCE(chain_id, id) for extra safety.

---

Summary: Excellent implementation with one user-facing bug that should be fixed. The markup injection issue can cause confusing output when diffing PRDs containing code or markdown.

[github-actions]

Review comment posted successfully. The PR has excellent implementation with one critical markup injection issue that should be fixed.

opencode session  |  github run

[github-actions]

Review Summary

This PR implements comprehensive PRD versioning functionality with excellent test coverage (68 new tests). The code is well-structured and follows the v2 headless architecture pattern. Most issues from prior reviews have been addressed, with only a few remaining items.

✅ Previously Addressed Issues

• ✅ Index on parent_id and chain_id added in workspace.py
• ✅ chain_id field implemented for version grouping
• ✅ list_chains() function added
• ✅ Transaction handling in create_new_version()
• ✅ Single-query version lookup using chain_id
• ✅ Dependency checking with PrdHasDependentTasksError
• ✅ Resilient database migrations with backfill
• ✅ Delete command checks return value and handles not-found case
• ✅ lineterm="\n" is correct with splitlines(keepends=True)

🐛 Remaining Issues

1. Rich Markup Conflict in Diff Output (Priority: High)
Location: codeframe/cli/app.py:990-997

The diff output uses inline Rich markup ([green]{line}[/green]) which can cause rendering issues when the diff content contains [ or ] characters (common in code snippets like print("hello") or [1, 2, 3]).

Fix: Use style= parameter instead of inline markup and add markup=False:

for line in diff.splitlines():
    if line.startswith("+") and not line.startswith("+++"):
        console.print(line, style="green", markup=False)
    elif line.startswith("-") and not line.startswith("---"):
        console.print(line, style="red", markup=False)
    elif line.startswith("@@"):
        console.print(line, style="cyan", markup=False)
    else:
        console.print(line, markup=False)

2. NULL chain_id Handling (Priority: Low)
Location: codeframe/core/prd.py:336

The list_chains() function uses INNER JOIN with p.chain_id = latest.chain_id. If chain_id is NULL (possible in legacy edge cases), rows will be dropped since NULL != NULL in SQL.

Fix: Use COALESCE(chain_id, id) for robust handling:

SELECT p.id, p.workspace_id, p.title, p.content, p.metadata, p.created_at,
       p.version, p.parent_id, p.change_summary, p.chain_id
FROM prds p
INNER JOIN (
    SELECT COALESCE(chain_id, id) as chain_key, MAX(version) as max_version
    FROM prds
    WHERE workspace_id = ?
    GROUP BY chain_key
) latest ON COALESCE(p.chain_id, p.id) = latest.chain_key AND p.version = latest.max_version
WHERE p.workspace_id = ?
ORDER BY p.created_at DESC

3. ROLLBACK Error Masking (Priority: Low)
Location: codeframe/core/prd.py:542-543

If cursor.execute("ROLLBACK") fails (e.g., connection closed), it will raise a new exception and mask the original error.

Fix: Wrap ROLLBACK in try/except:

except Exception:
    try:
        cursor.execute("ROLLBACK")
    except Exception:
        pass
    raise

📝 Overall Assessment

This is a high-quality PR with comprehensive functionality and excellent test coverage. The code follows v2 architecture principles well.

Recommended actions before merge:

1. Fix the Rich markup conflict (Issue Add Claude Code GitHub Workflow #1) - This is a real bug that could cause rendering problems

Nice to have (can be addressed in follow-up):
2. Add COALESCE handling for NULL chain_id (Issue #2)
3. Wrap ROLLBACK in try/except (Issue #3)

The implementation is solid and ready to merge with the markup fix applied.

[github-actions]

I've posted a comprehensive review to the PR. The review identifies:

1. High priority bug: Rich markup conflict in diff output that could cause rendering issues with code containing [ or ] characters
2. Low priority improvement: NULL chain_id handling in list_chains()
3. Low priority improvement: ROLLBACK error masking in exception handling

The PR is in excellent shape with comprehensive test coverage and clean architecture. Most prior issues have been addressed, with only the markup conflict requiring attention before merge.

opencode session  |  github run