refactor: replace string-based formula representation with interned SymbolIds and introduce stateful Clausifier for TPTP parsing

2 files changed, 54 insertions, 11 deletions

diff --git a/include/atp/normalize/clausifier.h b/include/atp/normalize/clausifier.h
index 1ff4d27..61d2505 100644
--- a/include/atp/normalize/clausifier.h
+++ b/include/atp/normalize/clausifier.h
@@ -16,15 +16,20 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-
 #pragma once
 
 /// @file clausifier.h
 /// @brief Converts FOL formulas to Clause Normal Form (CNF).
 ///
+/// Clausifier is a stateful object because:
+///   1. Skolem function counter must be shared across all formulas in a problem
+///      (otherwise different formulas generate conflicting Skolem names).
+///   2. Variable renaming counter must be shared for the same reason.
+///
 /// Pipeline: eliminate ↔/→  →  NNF  →  Skolemize  →  distribute ∨ over ∧  →  flatten.
 
 #include "atp/core/clause.h"
+#include "atp/core/symbol_table.h"
 #include "atp/core/term_bank.h"
 #include "atp/normalize/formula.h"
 
@@ -32,7 +37,33 @@
 
 namespace atp {
 
-/// Convert a first-order formula into a set of clauses (CNF).
-std::vector<Clause> clausify(const Formula& formula, TermBank& bank);
+/// Stateful clausifier. Create one per proof problem.
+///
+/// Usage:
+///   Clausifier clausifier(bank, symbols);
+///   auto clauses1 = clausifier.clausify(formula1);
+///   auto clauses2 = clausifier.clausify(formula2);
+///   // Skolem IDs are unique across both calls.
+class Clausifier {
+  public:
+    Clausifier(TermBank& bank, SymbolTable& symbols);
+
+    /// Convert a single formula to CNF clauses.
+    /// Skolem functions generated here are globally unique (counter persists).
+    [[nodiscard]] std::vector<Clause> clausify(const Formula& formula);
+
+    /// Negate a formula: wraps it in ¬(...)
+    /// Used to negate the conjecture before clausification.
+    [[nodiscard]] static std::unique_ptr<Formula> negate(std::unique_ptr<Formula> formula);
+
+    /// Number of Skolem functions generated so far.
+    [[nodiscard]] uint32_t skolemCount() const { return next_skolem_id_; }
+
+  private:
+    TermBank& bank_;
+    SymbolTable& symbols_;
+    uint32_t next_skolem_id_ = 0;
+    uint32_t next_var_id_ = 0;  ///< For variable standardization apart
+};
 
 }  // namespace atp
diff --git a/include/atp/normalize/formula.h b/include/atp/normalize/formula.h
index ccf1302..2cd7900 100644
--- a/include/atp/normalize/formula.h
+++ b/include/atp/normalize/formula.h
@@ -16,16 +16,19 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-
 #pragma once
 
 /// @file formula.h
 /// @brief First-order logic formula AST (pre-clausification).
+///
+/// Design decision: atoms use TermId (interned), quantifier variables use
+/// SymbolId (interned). The parser interns all symbols during parsing.
+/// This avoids string→ID lookups during clausification and ensures
+/// consistent variable identity across quantifier scopes.
 
 #include "atp/core/types.h"
 
 #include <memory>
-#include <string>
 #include <vector>
 
 namespace atp {
@@ -33,6 +36,7 @@ namespace atp {
 /// AST node types for first-order logic formulas.
 enum class FormulaKind : uint8_t {
     kAtom,       ///< Predicate application: P(t1, ..., tn)
+    kEquality,   ///< Equality atom: t1 = t2 (distinguished for equality reasoning)
     kNot,        ///< Negation: ¬F
     kAnd,        ///< Conjunction: F ∧ G
     kOr,         ///< Disjunction: F ∨ G
@@ -43,19 +47,27 @@ enum class FormulaKind : uint8_t {
 };
 
 /// A first-order logic formula node.
+///
+/// All identifiers are interned: atoms as TermIds, quantifier variables as
+/// SymbolIds. No raw strings survive past the parser.
 struct Formula {
     FormulaKind kind;
 
-    // For kAtom: the predicate term
+    /// For kAtom / kEquality: the predicate/equality term, already interned.
     TermId atom = kInvalidId;
 
-    // For quantifiers: the bound variable name
-    std::string var_name;
+    /// For kForall / kExists: the bound variable, as an interned SymbolId.
+    /// Using SymbolId (not string) so the clausifier can map bound variables
+    /// back to term-bank variables without string lookups.
+    SymbolId bound_var = kInvalidId;
 
-    // Sub-formulas (1 for Not/Quantifiers, 2 for binary connectives)
+    /// Sub-formulas (1 for Not/Quantifiers, 2 for binary connectives).
     std::vector<std::unique_ptr<Formula>> children;
 
+    // ── Factory methods ──
+
     static std::unique_ptr<Formula> makeAtom(TermId atom);
+    static std::unique_ptr<Formula> makeEquality(TermId lhs, TermId rhs, TermId eq_atom);
     static std::unique_ptr<Formula> makeNot(std::unique_ptr<Formula> child);
     static std::unique_ptr<Formula> makeAnd(std::unique_ptr<Formula> lhs,
                                             std::unique_ptr<Formula> rhs);
@@ -65,9 +77,9 @@ struct Formula {
                                                 std::unique_ptr<Formula> rhs);
     static std::unique_ptr<Formula> makeIff(std::unique_ptr<Formula> lhs,
                                             std::unique_ptr<Formula> rhs);
-    static std::unique_ptr<Formula> makeForall(std::string var,
+    static std::unique_ptr<Formula> makeForall(SymbolId var,
                                                std::unique_ptr<Formula> body);
-    static std::unique_ptr<Formula> makeExists(std::string var,
+    static std::unique_ptr<Formula> makeExists(SymbolId var,
                                                std::unique_ptr<Formula> body);
 };