Address code review: fix implicit global state between functions

Agent-Logs-Url: https://github.com/TUBAF-IfI-LiaScript/TUBAF-IfI-LiaScript.github.io/sessions/ba058e20-3bf9-4dba-b44b-66dfa10c52a7

Co-authored-by: vgoehler <1705385+vgoehler@users.noreply.github.com>

Author: Copilot

scripts/detect_changes.sh

@@ -15,9 +15,11 @@ get_changed_yamls() {
 
 # ---------------------------------------------------------------------------
 # check_courses – iterate all course YAMLs, collect which need regeneration.
+# $1: space-separated list of changed YAML files
 # Sets COURSES_TO_GENERATE and MISSING_HTML.
 # ---------------------------------------------------------------------------
 check_courses() {
+  local changed_yamls="$1"
   COURSES_TO_GENERATE=""
   MISSING_HTML=""
 
@@ -26,9 +28,9 @@ check_courses() {
     course_name=$(basename "$yaml_file" .yml)
     html_file="${course_name}.html"
 
-    if echo "$CHANGED_YAMLS" | grep -q "$yaml_file" || [ ! -f "$html_file" ]; then
+    if echo "$changed_yamls" | grep -q "$yaml_file" || [ ! -f "$html_file" ]; then
       echo "Course '$course_name' needs regeneration:"
-      if echo "$CHANGED_YAMLS" | grep -q "$yaml_file"; then
+      if echo "$changed_yamls" | grep -q "$yaml_file"; then
         echo "  - YAML file was changed"
       fi
       if [ ! -f "$html_file" ]; then
@@ -56,7 +58,7 @@ write_outputs() {
 main() {
   echo "=== Detecting Changes ==="
   get_changed_yamls
-  check_courses
+  check_courses "$CHANGED_YAMLS"
   write_outputs
 }
 

scripts/download_upstream_pdfs.sh

@@ -20,6 +20,11 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 # shellcheck source=courses_lib.sh
 . "${SCRIPT_DIR}/courses_lib.sh"
 
+# Global state shared between functions
+NAMES=()        # deduplicated PDF asset names (set by parse_pdf_assets)
+URLS=()         # corresponding download URLs  (set by parse_pdf_assets)
+declare -A CACHED_URL  # manifest cache: name → url (set by load_manifest)
+
 # ---------------------------------------------------------------------------
 # validate_args – check required arguments and resolve course→repo mapping
 # ---------------------------------------------------------------------------
@@ -107,7 +112,7 @@ parse_pdf_assets() {
   fi
 
   # Deduplicate: keep the first occurrence of each filename (= most-recent release)
-  declare -gA SEEN
+  declare -A SEEN
   NAMES=()
   URLS=()
   for i in "${!ALL_NAMES[@]}"; do
@@ -126,7 +131,6 @@ parse_pdf_assets() {
 # load_manifest – read the cached URL manifest into CACHED_URL associative array
 # ---------------------------------------------------------------------------
 load_manifest() {
-  declare -gA CACHED_URL
   if [ -f "$MANIFEST" ]; then
     while IFS=$'\t' read -r cached_name cached_url; do
       [[ -z "$cached_name" ]] && continue

scripts/generate_courses.sh

@@ -10,13 +10,13 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
 # ---------------------------------------------------------------------------
 # determine_pdf_needs – decide if PDF generation is required for a course.
-# Sets the global `needs_pdfs` variable (true/false).
+# Sets the caller-visible variable `needs_pdfs` to "true" or "false".
+# Must be called from a context where `needs_pdfs` is already declared.
 # ---------------------------------------------------------------------------
 determine_pdf_needs() {
   local course="$1"
   local yaml_file="${course}.yml"
   local manifest=".cache/${course}_upstream_pdfs"
-  needs_pdfs=false
 
   # Try to download upstream PDFs (creates/updates the manifest)
   if bash "$SCRIPT_DIR/download_upstream_pdfs.sh" "$course"; then
@@ -51,9 +51,11 @@ determine_pdf_needs() {
 
 # ---------------------------------------------------------------------------
 # run_liaex – run the liaex command for a given course.
+# $1: course name  $2: "true" if PDF generation is needed, "false" otherwise
 # ---------------------------------------------------------------------------
 run_liaex() {
   local course="$1"
+  local needs_pdfs="$2"
   local yaml_file="${course}.yml"
 
   case "$course" in
@@ -107,13 +109,14 @@ generate_course() {
 
   echo "Generating $html_file from $yaml_file..."
 
-  # Determine whether PDF generation is needed (skipped for index)
+  # Initialize needs_pdfs; determine_pdf_needs() will update it if applicable.
+  # Not declared local so that determine_pdf_needs can write to the same variable.
   needs_pdfs=false
   if [ "$course" != "index" ]; then
     determine_pdf_needs "$course"
   fi
 
-  run_liaex "$course"
+  run_liaex "$course" "$needs_pdfs"
 
   if [ -f "$html_file" ]; then
     echo "✅ Successfully generated $html_file"

scripts/prune_pdfs.sh

@@ -12,8 +12,10 @@ fi
 repo_root="$(cd "$(dirname "$0")/.." && pwd)"
 cd "$repo_root"
 
-# Associative array of PDF absolute paths to keep
+# Associative array of PDF absolute paths to keep (populated by collect_* functions)
 declare -A keep
+# Array of on-disk PDFs that are not referenced (populated by find_unreferenced)
+unreferenced=()
 
 # ---------------------------------------------------------------------------
 # collect_html_refs – scan HTML files and add referenced PDFs to `keep`