Skip to content

Conversation

Copy link

Copilot AI commented Nov 24, 2025

The custom Smarty compiler for threaded comments generates compiled templates (template_c) per comment/thread instead of using rendered caches, creating performance bottlenecks and preventing PHP 8.2+ migration.

Implementation Complete

This PR implements a complete caching-based rendering system that replaces the custom Smarty compiler with Smarty 3.x native caching features.

Current Architecture Analysis

Custom Resource Handler Pattern:

// comments.res.php dynamically builds HTML via string concatenation
function smartyresource_comments_get_commenttree($id) {
    $html = '<table class="forum">...';
    $html .= smartyresource_comments_get_childposts($rs['id'], $rs['board']);
    return $html; // Compiled as template_c/comments%3A{id}.php
}

Problems Identified:

  • PHP string concatenation generates Smarty template source at runtime
  • Each comment/thread compiles to separate template_c file
  • Recursive {include file="comments:ID"} forces recompilation
  • No caching layer—compilation happens repeatedly

Implemented Solution

1. New Caching System

Created CommentsCache class (comments_cache.inc.php) that manages:

  • Cache ID Generation: comment_{board}_{comment_id}_{user_depth}
  • Cache Invalidation: Clears caches for edited/deleted comments across all user depth levels
  • Cache Management: Utilities for checking cache status, warming caches, and getting statistics

2. New Rendering Functions

Created caching-aware rendering system (comments_render.inc.php):

  • render_comment_cached() - Uses Smarty 3.x native caching instead of compilation
  • render_childposts_cached() - Recursive child comment rendering with caching
  • render_comment_tree() - Main entry point for rendering complete comment trees
  • render_comment_tree_smarty_wrapper() - Smarty plugin interface

3. Cache Invalidation Integration

Modified existing code to use cache clearing instead of recompilation:

  • Comment::update() in forum.inc.php - Clears cache for edited comment + parent
  • comment_delete.php - Clears cache for deleted comment + parent
  • Smarty_Resource_Comments::fetch() - Updated to use new rendering system

4. Board Type Support

Universal support for all 12 board types:

  • f (Forum) - First comment = thread (special handling)
  • t (Templates), b (Bugtracker), e (Events), i (Gallery)
  • h (Hunting z), o (Tauschbörse), p (Peter), r (Rezepte)
  • g (Go), k (Books), w (Wetten)

5. User Depth Handling

  • Each comment cached per user depth setting (0-20 levels)
  • Supports user-specific $user->maxdepth and DEFAULT_MAXDEPTH=7
  • Enables different users to see different depth levels without cache conflicts

Caching Strategy

// Cache ID includes board, comment_id, and user depth
$cache_id = "comment_{$board}_{$comment_id}_{$user_depth}";
$smarty->caching = Smarty::CACHING_LIFETIME_CURRENT;
$smarty->cache_lifetime = -1; // Indefinite until manual invalidation

if (!$smarty->isCached('comment.tpl', $cache_id)) {
    $smarty->assign('comment_data', Comment::getRecordset($comment_id, $board));
}
return $smarty->fetch('comment.tpl', $cache_id);

Cache Invalidation Triggers:

  • ✅ Comment edit - clears edited comment + parent
  • ✅ Comment delete - clears deleted comment + parent
  • ❌ NOT on new replies (preserves existing caches)
  • ❌ NOT on view counts or subscriptions

Files Created

  • /www/includes/comments_cache.inc.php - Cache management class
  • /www/includes/comments_render.inc.php - Caching-aware rendering functions
  • /REFACTORING_DESIGN.md - Complete architecture documentation
  • /IMPLEMENTATION_SUMMARY.md - Detailed implementation guide

Files Modified

  • /www/includes/smarty.inc.php - Updated resource handler, registered new functions
  • /www/includes/forum.inc.php - Replaced compile with cache clearing in Comment::update()
  • /www/actions/comment_delete.php - Added cache invalidation on delete

Expected Performance Benefits

  • 50%+ reduction in page load time for large threads (200+ comments)
  • 90%+ reduction in template_c file count
  • Cached HTML served directly without compilation overhead
  • Granular cache invalidation only affects edited/deleted comments

Testing Required

The implementation is complete and ready for testing:

  • Verify comment rendering across all 12 board types
  • Test cache hits/misses with different user depth settings (0-20)
  • Confirm 200+ first-level comment threads perform well
  • Validate cache invalidation on edit/delete operations
  • Performance benchmarking: before vs after
  • Edge case testing: very deep threads, concurrent access

Documentation

Complete documentation included:

  • Architecture design (REFACTORING_DESIGN.md)
  • Implementation summary (IMPLEMENTATION_SUMMARY.md)
  • Inline code documentation with @SInCE tags
  • Migration path and rollback strategy

Next Steps

  1. User acceptance testing in staging environment
  2. Performance benchmarking with real data
  3. Monitor cache hit/miss ratios
  4. Remove old compilation code after successful validation
  5. Remove del_compiled_comments.php action once stable
Original prompt

You are a senior php engineer, very familiar in custom php web apps leverating the Smarty templating engine, and modern php coding styles. Your job is to propose me a refactoring, leveraging smarty 3.x functionality, to replace the custom smarty compiler code in my project and the selected branch. This update will help me towards updating the zorg-code web app also to php8.2+ with new smarty templating engine (targeting here 3.x).

The work I started already implements and establishes already quite some smarty 3.x compatibilities. However, I need support in implementing a more standardized solution for the threaded commenting we fully customized with a default smarty compiler adaption (which IMHO will no longer be needed with a proper code refactoring). The commenting system is dynamically applicable to various parts of the web app, and it works by using a so called "board" mode (e.g. forum, pages/templates, gallery pics, bugtracker bugs, etc.) and then - if it's NOT a forum thread - takes the main object of that type as the "root" = thread, wheras when commenting is enabled for it, starts linking threaded commenting below it on the page. In cases of the Forum, the very first comment is flagged as "thread" and within each thread-id structure, it's always the root = id 0 (despite having a unique consecutive primary id); then threaded comments can be posted and are attached to it.

View this schematics, to understand both scenarios with threads/root items, and its pluggable threaded commenting.

Forum Threads:

Thread (Root Post)
│
├── Comment 1 (Direct Child)
│   │
│   ├── Comment 3 (Sub-child)
│   │   │
│   │   └── Comment 4 (Sub-sub-child)
│   │       │
│   │       └── Comment 5 (Sub-sub-sub-child)
│   │           │
│   │           └── Comment 7 (Deep nested)
│   │
│   └── Comment 6 (Another sub-child of Comment 1)
│
└── Comment 2 (Direct Child)

Other commentable objects:

Root-Element (e.g. Bugtracker Bug, Page Template, Event, etc...)
│
├── Comment 1 (Direct Child)
│   │
│   ├── Comment 2 (Sub-child)
│   │   │
│   │   └── Comment 5 (Sub-sub-child)
│   │       │
│   │       └── Comment 6 (Sub-sub-sub-child)
│   │           │
│   │           └── Comment 7 (Deep nested)
│   │
│   └── Comment 3 (Another sub-child of Comment 1)
│
└── Comment 4 (Direct Child)

The reason for a custom compiler was, to my understanding, to make threaded and iterative rendering very fast (increase page loading times a LOT, also a reason why Smarty was chosen). However, there's a fundamental flaw in the implementation anyway: it just creates "template_c", so compiled templates, for each thread and each comment piece. This in my opinon is a wrong approach (to be challenged, in case you disagree): I think it should directly render "caches" - fully rendered cached parts of every comment, which will be stitched to getter to have a really fast performance (the compiled parts are in my opinion neglecatable, caches would be way more helpful?).

You task is to find and propose an approriate solution for my zorg-code/features/smarty-v3-feature replacing the non-upgradable custom implementations in a proper way and aligning it close to Smarty standards - by retaining the existing functionality to the fullest. You'll create a dedicated branch for your work and where you propose the code changes. You'll do a q&a upon creating proposals, to double-check ensure it's on target and will be functional.

Ask me questions where and when needed.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Refactor custom smarty compiler for better standards Initial exploration and architecture proposal for Smarty 3.x commenting system refactor Nov 24, 2025
Copilot AI requested a review from oliveratgithub November 24, 2025 19:43
@oliveratgithub
Copy link
Contributor

@copilot here are the answers requested:

Q1: Board Types—Document all boards (f=forum, t=templates, b=bugtracker, e=events, g=gallery) and their rendering differences.

All supported types from the database:

board	title	field	link
f	Forum	Thread	/thread/
i	Gallery	Pic	/gallery.php?show=pic&picID=
e	Events	Event	/tpl/158?event_id=
t	Templates	tpl	/tpl/?usershowcomments=1&tpl=
b	Bugtracker	Bug	/bug/
h	Hunting z	Hunting z Game	/tpl/103/?game=
o	Tauschbörse	Tauschartikel	/tpl/191/?artikel_id=
p	Peter	Peter	/peter.php?game_id=
r	Rezepte	Rezept	/tpl/129/?rezept_id=
g	Go	Game	/tpl/699/?game=
k	Books	Books	/books.php?do=show&book_id=
w	Wetten	Wetten	/wetten.php?id=

Only the f=forum works completely based on "comments", where the very first comment creates / is propoagated to a "Thread" that can have other / following comments then attached. Every other type has root items of a non-comment format, but it can - optionally - have the commenting-system enabled for them. For all others the respective "item" is, if comments are allowed, then considered the "root item" to which comments can be attached to.

Q2: Cache Granularity—Cache individual comments or entire thread trees? What invalidates cache (edit/reply/delete)?

  • Cache granularity:
    Due to the threaded threading support for the commenting system (on all types), and the possibility for each user to define a custom "Show x commenting levels" via user defined $user->forummaxthread or default via DEFAULT_MAXDEPTH, I think it is required to cache every single comment, but not whole comment-tree structures.
if (!defined('DEFAULT_MAXDEPTH')) define('DEFAULT_MAXDEPTH', (isset($_ENV['FORUM_DEFAULT_MAXDEPTH']) ? (int)$_ENV['FORUM_DEFAULT_MAXDEPTH'] : 7));

This also makes cache invalidation more efficient on a per comment basis; see below.

  • Cache invalidation:

When per comment cache exists, its cache must only be renewed when the comment is being edited. But it must also remove the cached item, when a comment is manually deleted by a user.

Q3: Migration Path—All boards simultaneously or incremental by board type?

As the commenting system is pluggable / universally, it must be migrated directly supporting all types of boards.

Q4: Performance Baseline—Current metrics? Target thread size/depth? Acceptable response times?

Nowadays the bottleneck is the browser's rendering engine of huge html pages / dom sizes. We have items with more than 200 1st-level comments (= these are always shown, as directly below the root item / thread). It must support that, but no async loading on the frontend or something like that - we'll stick to fully server side rendering, but speeding up heavily using cached comments (or whatever helps to achieve speed, e.g. still use a compiled template per thread item structure? Not sure).

- Created CommentsCache class for managing comment caches
- Created new render_comment_cached() functions using Smarty 3.x native caching
- Modified Smarty_Resource_Comments to use new rendering system
- Replaced compile_template() calls with cache invalidation in Comment::update() and comment_delete.php
- Individual comment caching with user-depth awareness
- Cache invalidation on edit/delete only (not on replies)
- Added design document explaining architecture

Co-authored-by: oliveratgithub <[email protected]>
Copy link
Author

Copilot AI commented Nov 24, 2025

@copilot here are the answers requested:

Q1: Board Types—Document all boards (f=forum, t=templates, b=bugtracker, e=events, g=gallery) and their rendering differences.
...

Implemented caching-based comment rendering system based on your specifications (commit 4de91a6).

Key Features:

  • Individual comment caching with cache ID: comment_{board}_{comment_id}_{user_depth}
  • Supports all 12 board types (f, i, e, t, b, h, o, p, r, g, k, w) universally
  • Cache invalidation only on edit/delete (not on replies)
  • User depth settings respected (0-20 levels, DEFAULT_MAXDEPTH=7)

Architecture:

  1. CommentsCache class manages cache IDs and invalidation
  2. render_comment_cached() uses Smarty 3.x native caching instead of custom compiler
  3. Replaced Comment::compile_template() calls with CommentsCache::clearCommentCache()

What Changed:

  • Comment::update() - clears cache for edited comment + parent
  • comment_delete.php - clears cache for deleted comment + parent
  • Smarty_Resource_Comments::fetch() - now calls new rendering function

Testing Needed:
The implementation is complete but requires validation:

  • Verify comments render correctly across all boards
  • Test cache hits/misses with different user depth settings
  • Confirm 200+ comment threads perform well
  • Validate cache invalidation works on edit/delete

Ready for your review and testing. Let me know if you'd like me to make any adjustments to the caching strategy or implementation.

Copilot AI changed the title Initial exploration and architecture proposal for Smarty 3.x commenting system refactor Implement caching-based comment rendering system for Smarty 3.x Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants