feat: Add table of contents generator for manager book

[idvorkin-ai-tools]

Summary

• Created Python script to generate merged table of contents for manager book posts
• Added pre-commit hook to automatically update TOC when manager book files change
• Updated both manager book and appendix with generated TOC

Implementation Details

• TOC combines content from both main post and appendix into single navigation
• Links automatically adjust based on context (main page vs appendix)
• Pre-commit hook only runs on manager book files for efficiency

Test Plan

• Tested script with python3 scripts/generate_toc.py check
• Verified TOC generation with python3 scripts/generate_toc.py manager-book
• Confirmed both files updated with merged TOC
• Pre-commit hook configured to run only on manager book files

Fixes #160

🤖 Generated with Claude Code

[claude]

Claude finished @idvorkin-ai-tools's task —— View job

---

PR Review: Table of Contents Generator

I've completed a thorough review of this PR. Here's my comprehensive feedback:

Todo List:

• Analyze the Python script scripts/generate_toc.py
• Review pre-commit hook configuration changes
• Examine generated TOC content in both files
• Check for security concerns and best practices
• Assess test coverage and suggest improvements
• Provide comprehensive feedback and recommendations

✅ Strengths

Well-structured Python code: The script follows good Python practices with type hints, clear function separation, and comprehensive docstrings.

Proper Jekyll integration: The script correctly handles Jekyll front matter and respects existing TOC markers (and).

Smart anchor generation: The extract_headers function correctly mimics Jekyll/GitHub anchor generation with proper character sanitization.

Efficient pre-commit integration: The hook only triggers on the specific manager book files, avoiding unnecessary runs.

🔧 Issues & Recommendations

1. Code Quality & Bug Fixes

Function signature mismatch in scripts/generate_toc.py:40:

• Function expects 4-tuple (level, text, anchor, url_prefix) but extract_headers returns 3-tuple (level, text, anchor)
• This will cause runtime errors

Regex vulnerability in scripts/generate_toc.py:75-80:

• Using re.sub() with user-controlled content could be exploited
• Consider using re.escape() for the patterns or validate input

Error handling gaps:

• Path.read_text() calls don't handle encoding issues or permission errors
• Path.write_text() doesn't handle write failures gracefully

2. Performance Considerations

File I/O optimization:

• Files are read multiple times unnecessarily
• Could cache content after first read to improve performance

Regex compilation:

• header_pattern is compiled on every call to extract_headers
• Should compile once as a module-level constant

3. Security Concerns

Path traversal potential:

• Hard-coded paths are safe, but consider validating if this becomes configurable
• File operations lack permission checks

Input validation:

• No validation of markdown content before regex processing
• Large files could cause memory issues

4. Test Coverage - Critical Gap

No unit tests present:

• Complex logic in generate_merged_toc and update_manager_book_toc is untested
• Regex patterns should be tested with edge cases
• File I/O operations need error condition testing

Suggested test structure:

# tests/test_generate_toc.py
def test_extract_headers_basic()
def test_extract_headers_edge_cases()  
def test_generate_toc_with_url_prefix()
def test_anchor_generation_special_chars()
def test_update_existing_toc()
def test_insert_new_toc()

5. Pre-commit Hook Analysis

Configuration looks solid in .pre-commit-config.yaml:78-84:

• Correctly targets only manager book files
• Uses proper regex pattern for file matching
• Runs without filename passing (appropriate for this use case)

📋 Immediate Action Items

1. Fix function signature bug - Critical for functionality
2. Add comprehensive unit tests - Essential for maintainability
3. Improve error handling - File operations should be more robust
4. Security hardening - Escape regex patterns and validate inputs
5. Performance optimization - Cache file reads and compile regex patterns

📊 Overall Assessment

This PR implements a valuable feature that will keep the manager book TOCs synchronized automatically. The core logic is sound and the integration approach is well thought out. However, the critical bug in the function signature and lack of test coverage need to be addressed before merging.

Recommendation: Request fixes for the critical bug and addition of unit tests before approval.

The implementation shows good understanding of Jekyll, markdown processing, and automation principles. With the suggested improvements, this will be a robust addition to the content management workflow.

[claude]

Claude finished @idvorkin-ai-tools's task —— View job

---

PR Review: Table of Contents Generator

I'm conducting a comprehensive review of this PR focusing on code quality, potential bugs, performance, security, and test coverage.

Todo List:

• Analyze the Python script scripts/generate_toc.py
• Review pre-commit hook configuration changes
• Examine generated TOC content in both files
• Check for security concerns and best practices
• Assess test coverage and suggest improvements
• Provide comprehensive feedback and recommendations

✅ Strengths

Well-structured Python code: The script follows good Python practices with type hints, clear function separation, and comprehensive docstrings.

Proper Jekyll integration: The script correctly handles Jekyll front matter and respects existing TOC markers (``).

Smart anchor generation: The extract_headers() function correctly mimics Jekyll/GitHub anchor generation with proper character sanitization.

Efficient pre-commit integration: The hook only triggers on the specific manager book files, avoiding unnecessary runs.

🚨 Critical Issues

1. Function Signature Mismatch (Runtime Bug)

Location: scripts/generate_toc.py:40-47

The generate_toc() function expects a 4-tuple (level, text, anchor, url_prefix) but extract_headers() returns a 3-tuple (level, text, anchor). This will cause a ValueError: not enough values to unpack when the script runs.

Fix needed: Either modify extract_headers() to return 4-tuples or update generate_toc() to handle 3-tuples.

2. Regex Security Vulnerabilities

Location: scripts/generate_toc.py:75-80, 159-160

Using re.sub() with user-controlled content creates potential ReDoS (Regular Expression Denial of Service) vulnerabilities:

# Potentially dangerous - user content in regex
main_clean = re.sub(f"{toc_start_pattern}.*?{toc_end_pattern}", "", main_content, flags=re.DOTALL)

Recommendation: Use re.escape() for patterns or validate input length/complexity.

🔧 Code Quality Issues

3. Missing Error Handling

• Path.read_text() calls don't handle encoding issues or file permission errors
• Path.write_text() operations lack error handling for write failures
• Large files could cause memory issues with no size validation

4. Performance Inefficiencies

• Files are read multiple times unnecessarily (lines 72-73, 137)
• header_pattern regex is compiled on every call to extract_headers() (line 22)
• No caching of file content after first read

5. Code Logic Issues

Location: scripts/generate_toc.py:114-128

The appendix linking logic has potential issues:

• Assumes case-insensitive matching for "Appendix" header detection
• All appendix items link to #appendix regardless of whether that section exists

📋 Missing Test Coverage - Critical Gap

No unit tests present for this complex script. The following functionality is untested:

• Header extraction with edge cases (malformed headers, special characters)
• TOC generation with various URL prefix scenarios
• File I/O error conditions
• Regex anchor generation edge cases
• Merged TOC logic with different file structures

Recommended test structure:

# tests/test_generate_toc.py
def test_extract_headers_basic()
def test_extract_headers_special_chars()
def test_generate_toc_with_url_prefix()
def test_anchor_generation_edge_cases()
def test_update_existing_toc()
def test_insert_new_toc()
def test_merged_toc_generation()
def test_file_not_found_errors()

✅ Pre-commit Configuration Analysis

Location: .pre-commit-config.yaml:78-84

The pre-commit hook configuration is well-implemented:

• ✅ Correctly targets only manager book files with regex pattern
• ✅ Uses pass_filenames: false (appropriate for this script)
• ✅ Proper language: system setting
• ✅ Efficient file filtering to avoid unnecessary runs

🔒 Security Assessment

Medium Risk:

• Path traversal potential if paths become configurable
• No input validation on file size or content complexity
• Regex patterns could be exploited for ReDoS attacks

Recommendations:

1. Add input validation (file size limits, content complexity checks)
2. Use re.escape() for user-controlled regex content
3. Consider timeout limits for regex operations

📊 Overall Assessment

This PR implements a valuable automation feature with sound architecture. However, the critical runtime bug must be fixed before merging. The implementation shows good understanding of Jekyll, markdown processing, and git workflow integration.

Immediate Actions Needed:

1. 🚨 Fix function signature mismatch - Critical for functionality
2. 🧪 Add comprehensive unit tests - Essential for maintainability
3. 🔒 Address security vulnerabilities - Important for production use
4. ⚡ Optimize performance - File I/O and regex compilation improvements

Recommendation: Request fixes for the critical bug and addition of unit tests before approval. The feature concept is excellent but needs these technical issues resolved.

---