vivaglint-py — Session Changelog
=================================
Session date : 2026-03-31 / 2026-04-01
Starting state: 212 tests, 8 open bugs (Medium+), 0 Critical/High open
Ending state  : 271 tests, 5 open bugs (Medium), 0 Critical/High open
                5 known failures documented as xfail in test suite


================================================================================
PART 1 — SOURCE CODE CHANGES
================================================================================

Each entry lists: file changed, what changed, which bug it fixes, and the
exact location so the diff can be found in git or code review.

--------------------------------------------------------------------------------
1.1  vivaglint/utils.py — Bug 10 fix
--------------------------------------------------------------------------------
Function : mean_to_glint_score()
Change   : Added guard before the division:

    if scale_points < 2:
        raise ValueError(f"scale_points must be >= 2, got {scale_points!r}")

Why      : scale_points = 1 makes the denominator (scale_points - 1) = 0,
           raising ZeroDivisionError instead of a descriptive ValueError.
           The docstring stated the valid range was 2-11 but the function
           itself had no enforcement.
Location : vivaglint/utils.py, inside mean_to_glint_score(), before the
           return statement.

--------------------------------------------------------------------------------
1.2  vivaglint/analyze.py — Bug 3 fix
--------------------------------------------------------------------------------
Function : analyze_attrition()
Change   : Removed the deprecated keyword argument from pd.to_datetime():

    Before:  pd.to_datetime(attrition_data[term_date_col],
                            infer_datetime_format=True)
    After:   pd.to_datetime(attrition_data[term_date_col])

Why      : pandas deprecated infer_datetime_format in 2.x and will remove it
           in a future release.  Strict parsing is now the default, so the
           argument is unnecessary and generates a FutureWarning on every call
           to analyze_attrition().
Location : vivaglint/analyze.py, inside analyze_attrition(), date parsing block.

--------------------------------------------------------------------------------
1.3  vivaglint/analyze.py — Bug 12 fix
--------------------------------------------------------------------------------
Function : analyze_by_attributes()
Change   : Replaced chained view assignment with an explicit .copy():

    Before:  group_data = survey_data[mask]
             slim_df = group_data[available_standard + available_question]
    After:   slim_df = survey_data.loc[mask,
                           available_standard + available_question].copy()

Why      : Under pandas 2.x, chained boolean indexing followed by column
           indexing produces SettingWithCopyWarning.  Under pandas 3.0
           Copy-on-Write, any mutation of slim_df would silently fail.
           The .copy() makes intent explicit and future-proofs the code.
Location : vivaglint/analyze.py, inside analyze_by_attributes(), per-group loop.
           The group_data local variable was removed (now unused).

--------------------------------------------------------------------------------
1.4  vivaglint/analyze.py — Bug 19 fix (get_response_dist)
--------------------------------------------------------------------------------
Function : get_response_dist()
Change   : Added emp_id_col parameter and forwarded it to _resolve_survey():

    Before signature:  def get_response_dist(survey, questions="all",
                                             plot=False)
    After signature:   def get_response_dist(survey, questions="all",
                                             plot=False,
                                             emp_id_col=None)

    Before call:  data, all_questions, _ = _resolve_survey(survey)
    After call:   data, all_questions, _ = _resolve_survey(survey, emp_id_col)

Why      : The docstring stated the function accepts a plain DataFrame, but
           _resolve_survey(survey) called extract_questions(data, None) when
           given a plain DataFrame, raising ValueError about missing emp_id_col.
           The API contract was broken; this fix fulfils it.
Location : vivaglint/analyze.py, get_response_dist() definition and first
           _resolve_survey() call (~line 256).

--------------------------------------------------------------------------------
1.5  vivaglint/analyze.py — Bug 19 fix (get_correlations)
--------------------------------------------------------------------------------
Function : get_correlations()
Change   : Same pattern as 1.4 — emp_id_col parameter added:

    Before signature:  def get_correlations(survey, method="pearson",
                                            format="long", use="pairwise",
                                            plot=False)
    After signature:   ... emp_id_col=None added as last parameter

    Before call:  data, questions, _ = _resolve_survey(survey)
    After call:   data, questions, _ = _resolve_survey(survey, emp_id_col)

Why      : Same broken API contract as get_response_dist (both in Bug 19).
Location : vivaglint/analyze.py, get_correlations() definition and the
           _resolve_survey() call inside it (~line 491).

--------------------------------------------------------------------------------
1.6  vivaglint/analyze.py — Bug 24 fix (deepcopy)
--------------------------------------------------------------------------------
Function : analyze_by_attributes()
Change   : Upgraded shallow copy to deep copy:

    Before:  survey = join_attributes(copy.copy(survey), attribute_file, ...)
    After:   survey = join_attributes(copy.deepcopy(survey), attribute_file, ...)

Why      : copy.copy() creates a shallow GlintSurvey copy but the metadata
           dict is the SAME object in both the original and the copy.  When
           join_attributes() wrote to survey.metadata["attribute_cols"], it
           modified the caller's original survey metadata — exactly the
           mutation Bug 2 intended to prevent.  copy.deepcopy() creates a
           fully independent copy including the metadata dict.
           Discovered by test_regression.py::TestBug2 during this session.
Location : vivaglint/analyze.py, inside analyze_by_attributes(), attribute
           file joining block (~line 883).

Note     : Bug 9 (NaT AttributeError) was investigated but found to be a
           non-issue.  pd.notna() correctly handles both NaT and None in the
           existing lambda; an attempted fix using "is not None" broke tests
           and was reverted.  The bug was reclassified as "NOT A BUG" in
           bugs.txt.


================================================================================
PART 2 — NEW TEST FILES
================================================================================

Two new test files were written and added to tests/.  Together they add 64 new
tests and bring the total from 212 to 271 (plus 5 xfail).

--------------------------------------------------------------------------------
2.1  tests/test_regression.py  (25 tests, 0 xfail)
--------------------------------------------------------------------------------
Purpose : Prevent fixed bugs from being reintroduced.  One class per bug.
          Every class docstring identifies the bug number and summarises what
          broke and how it was fixed.

Classes and tests:

  TestBug2AnalyzeByAttributesDoesNotMutateSurvey  (2 tests)
    test_original_column_count_unchanged
      — analyze_by_attributes(attribute_file=...) must not add columns to
        survey.data on the original object.
    test_original_attribute_cols_metadata_unchanged
      — survey.metadata["attribute_cols"] must remain [] after the call.
        This test caught Bug 24 (shallow copy) during this session.

  TestBug5PivotLongMissingSubColumn  (3 tests)
    test_missing_comment_col_does_not_raise
      — pivot_long must not raise when a _COMMENT column is absent.
    test_missing_comment_col_fills_na
      — Rows for the affected question must have NA in the comment column.
    test_missing_topics_col_fills_na
      — Same for the _COMMENT_TOPICS column.

  TestBug6CompareCyclesLexicographicSort  (2 tests)
    test_first_cycle_always_has_nan_change
      — Uses cycle_names=["Cycle 1","Cycle 2","Cycle 10"] (which lexicographic
        sort mis-orders).  "Cycle 1" must still have NaN change.
    test_last_cycle_change_references_correct_predecessor
      — "Cycle 10" change must equal mean(Cycle10)-mean(Cycle2), not
        mean(Cycle10)-mean(Cycle1).

  TestBug7JoinAttributesNumericEmpId  (1 test)
    test_numeric_emp_id_merge_matches_rows
      — Builds a survey with integer EMP IDs and attributes with string IDs.
        After join_attributes(), Department must not be all NaN.

  TestBug8ExtractSurveyFactorsTooFewRows  (3 tests)
    test_all_missing_one_column_raises_value_error
    test_single_complete_row_raises_value_error
    test_error_message_mentions_complete_rows
      — All verify that extract_survey_factors() raises a descriptive
        ValueError (not a numpy crash) when clean_data has < 2 rows.

  TestBug10MeanToGlintScoreZeroDivision  (5 tests)
    test_scale_points_1_raises_value_error
    test_scale_points_0_raises_value_error
    test_scale_points_minus_1_raises_value_error
    test_scale_points_2_does_not_raise
    test_all_valid_scale_points_work
      — Full coverage of the guard condition added by the Bug 10 fix.

  TestBug12AnalyzeByAttributesNoCopyWarning  (1 test)
    test_no_setting_with_copy_warning
      — Promotes SettingWithCopyWarning to an error; analyze_by_attributes()
        must complete without raising it.

  TestBug19PlainDataFrameSupport  (4 tests)
    test_get_response_dist_plain_dataframe
    test_get_correlations_plain_dataframe
      — Both functions must accept a plain DataFrame with emp_id_col provided.
    test_get_response_dist_without_emp_id_raises
    test_get_correlations_without_emp_id_raises
      — Without emp_id_col the error must mention "emp_id_col", not raise an
        internal AttributeError.

  TestBug23OblimineWarning  (5 tests)
    test_oblimin_issues_user_warning
    test_quartimin_issues_user_warning
      — Both must emit a UserWarning mentioning "Bug 23".
    test_varimax_does_not_warn
    test_promax_does_not_warn
      — Stable rotations must not emit any UserWarning.
    test_default_rotation_is_varimax
      — Calling extract_survey_factors() with no rotation argument must not
        trigger the Bug 23 warning.

--------------------------------------------------------------------------------
2.2  tests/test_edge_cases.py  (39 tests, 5 xfail)
--------------------------------------------------------------------------------
Purpose : Cover scenarios not exercised by the primary test files.  xfail tests
          document known open bugs so they are tracked inside the test suite.

Classes and tests:

  TestSplitSurveyDataWithAttributes  (3 tests, 2 xfail)   [Bug 26]
    test_attribute_cols_not_in_quantitative  (xfail)
      — "Department" must not appear in split["quantitative"].
    test_quantitative_has_exactly_n_question_cols  (xfail)
      — Quantitative split must have exactly N_QUESTIONS response columns.
    test_row_counts_still_correct
      — Row counts (1000 each) must be correct regardless of column bug.

  TestExtractQuestionsAttributeEnriched  (2 tests, 1 xfail)   [Bug 26]
    test_attribute_cols_not_returned_as_questions  (xfail)
      — extract_questions(enriched_glintsurvey) must not return "Department"
        as a question stem.
    test_question_count_from_metadata_is_correct
      — summarize_survey() uses metadata and returns the correct 10 questions
        even on an enriched survey (confirms analysis functions are unaffected).

  TestCompareCyclesPctChangeInf  (1 test)
    test_zero_previous_mean_produces_inf
      — Verifies current behaviour: dividing by zero mean produces inf, not
        NaN.  Locks in the behaviour documented by Bug 13.

  TestPivotLongNonDefaultIndex  (3 tests)
    test_filtered_survey_no_nan_rows
    test_filtered_survey_correct_row_count
    test_response_values_match_original
      — All three verify that pivot_long handles surveys whose .data has a
        non-zero-based index (e.g. after slicing) without row misalignment.

  TestPivotLongIncludeEmpty  (3 tests)
    test_include_empty_true_has_more_rows_than_false
    test_include_empty_true_all_returns_same_count
    test_both_dict_include_empty_true
      — Verifies data_type="comments" with include_empty=True behaves as
        documented.

  TestJoinAttributesAccumulatesMetadata  (2 tests)
    test_second_join_adds_new_columns
      — Calling join_attributes twice with different attribute files must
        accumulate both column sets in metadata["attribute_cols"].
    test_repeated_join_no_duplicate_metadata
      — Joining the same file twice must not duplicate entries in metadata.

  TestGetAllReportsEdgeCases  (2 tests, 1 xfail)
    test_leaf_employee_returns_empty_list
      — An employee who never appears as a Manager ID must return [].
    test_result_excludes_manager_itself  (xfail)   [Bug 25]
      — The queried manager must not appear in their own report list.
        Currently fails when the org chart has a cycle.

  TestAggregateByManagerUnknownManager  (1 test)
    test_manager_not_in_survey_has_empty_name
      — Manager IDs that don't appear in EMP ID (not survey respondents)
        must produce an empty string for manager_name, not NaN or an error.

  TestExtractSurveyFactorsNFactorsBoundaries  (6 tests)
    test_n_factors_zero_raises
    test_n_factors_too_large_raises
    test_n_factors_1_returns_single_factor
    test_n_factors_max_works
    test_rotation_none_returns_unrotated_solution
    test_promax_rotation_returns_valid_loadings

  TestSummarizeSurveyAllNaN  (2 tests)
    test_all_nan_question_returns_nan_mean
    test_all_nan_question_n_responses_is_zero
      — A fully-skipped question must return NaN mean/sd and n_responses=0.

  TestSearchCommentsSpecificQuestion  (2 tests)
    test_search_returns_question_column
    test_no_comment_col_in_data_returns_empty
      — search_comments on a survey with no _COMMENT columns must return an
        empty DataFrame, not raise.

  TestGetResponseDistPctRounding  (2 tests, 1 xfail)   [Bug 20]
    test_pct_values_rounded_to_1dp  (xfail)
      — get_response_dist() pct columns should be rounded to 1 dp like
        summarize_survey(); currently they are full-precision floats.
    test_pct_values_are_floats
      — Documents the current (unrounded) dtype behaviour.

  TestCompareCyclesGlintScoreChange  (2 tests)
    test_glint_score_change_formula
      — glint_score_change_from_previous must equal gs(cycleN)-gs(cycleN-1).
    test_first_cycle_glint_score_change_is_nan

  TestValidateGlintStructureOrphans  (2 tests)
    test_orphaned_comment_col_raises
      — A _COMMENT column with no matching base column must raise ValueError.
    test_complete_set_passes

  TestGlintSurveyMetadataIntegrity  (5 tests)
    test_questions_df_has_correct_types
    test_n_respondents_matches_data_length
    test_n_questions_matches_questions_df
    test_standard_columns_list_has_no_duplicates
    test_file_path_is_absolute


================================================================================
PART 3 — NEW BUGS DISCOVERED AND DOCUMENTED
================================================================================

Three new bugs were added to bugs.txt during this session.

--------------------------------------------------------------------------------
Bug 24 — HIGH — analyze_by_attributes shallow copy leaves metadata shared
--------------------------------------------------------------------------------
Discovered : test_regression.py::TestBug2::test_original_attribute_cols_metadata_unchanged
Status     : FIXED in this session (copy.deepcopy applied)
Root cause : copy.copy(GlintSurvey) copies the object but not the metadata dict.
             join_attributes() writing to survey.metadata["attribute_cols"]
             modified the caller's original survey because both the copy and
             the original pointed at the same dict.
Fix        : analyze_by_attributes(): copy.copy(survey) → copy.deepcopy(survey)

--------------------------------------------------------------------------------
Bug 25 — MEDIUM — get_all_reports can return the manager in their own report list
--------------------------------------------------------------------------------
Discovered : test_edge_cases.py::TestGetAllReportsEdgeCases::test_result_excludes_manager_itself
Status     : Open (xfail in test suite)
Root cause : The cycle guard in _get_all_reports_impl prevents re-PROCESSING a
             visited node but does not prevent it from being added to all_reports
             as a direct report of a descendant that reports back to it.
             The fixture data contains at least one such cyclic relationship.
Fix needed : Filter the final deduplicated list to exclude the original manager_id.

--------------------------------------------------------------------------------
Bug 26 — MEDIUM — split_survey_data and extract_questions pollute results with
                  attribute columns on enriched surveys
--------------------------------------------------------------------------------
Discovered : test_edge_cases.py::TestSplitSurveyDataWithAttributes (xfail)
             test_edge_cases.py::TestExtractQuestionsAttributeEnriched (xfail)
Status     : Open (xfail in test suite)
Root cause : split_survey_data() calls extract_questions(data, emp_id_col)
             where data is the raw (enriched) DataFrame.  extract_questions()
             only excludes get_standard_columns(), which does not include joined
             attribute columns like "Department".  These then appear as question
             stems and end up in the quantitative output as response columns.
             Same issue in direct calls to extract_questions(enriched_survey).
Note       : Analysis functions (summarize_survey, get_correlations, etc.) are
             NOT affected — they use _resolve_survey() which reads questions from
             survey.metadata["questions"], set before attributes were joined.
Fix needed : split_survey_data() should pass the GlintSurvey (not its .data) to
             extract_questions(), or extract_questions() should exclude columns
             listed in survey.metadata["attribute_cols"].


================================================================================
PART 4 — BUG STATUS SUMMARY AT END OF SESSION
================================================================================

ID   Priority  Status
---  --------  ----------------------------------------------------------------
1    Critical  FIXED — extract_survey_factors FactorAnalyzer crash
2    High      FIXED — analyze_by_attributes mutates caller's GlintSurvey
               (deepcopy fix applied this session after Bug 24 found)
3    Medium    FIXED — infer_datetime_format deprecation warning
4    Low       Open  — join_attributes in-place mutation documentation gap
5    Critical  FIXED — pivot_long scalar pd.NA crash
6    Critical  FIXED — compare_cycles lexicographic cycle sort
7    High      FIXED — join_attributes numeric emp_id silent NaN
8    High      FIXED — extract_survey_factors empty data numpy crash
9    Medium    NOT A BUG — pd.notna() already handles NaT correctly
10   Medium    FIXED — mean_to_glint_score ZeroDivisionError
11   Medium    RESOLVED by Bug 8 fix
12   Medium    FIXED — analyze_by_attributes SettingWithCopyWarning
13   Low       Open  — compare_cycles inf pct_change (documented by test)
14   Low       Open  — hierarchy NaN manager silently excluded
15   Low       Open  — search_comments empty result dtype inconsistency
16   Low       Open  — split_survey_data verbose deduplication (code smell)
17   Low       Open  — analyze_by_attributes float scale_points confusing error
18   Low       Open  — pivot_long non-default index (tested; passes currently)
19   Medium    FIXED — get_response_dist / get_correlations reject plain DFs
20   Low       Open  — get_response_dist pct values unrounded (xfail test)
21   Low       FIXED — extract_survey_factors stale docstring
22   Low       Open  — test robustness gap
23   Medium    PARTIALLY MITIGATED — oblimin degenerate solutions; default
               changed to varimax; UserWarning added (xfail test)
24   High      FIXED — analyze_by_attributes shallow copy metadata mutation
25   Medium    Open  — get_all_reports returns manager in own report list
               (xfail test)
26   Medium    Open  — split_survey_data / extract_questions include attribute
               cols as question stems (xfail test)


================================================================================
PART 5 — TEST COUNTS
================================================================================

                        Before this session    After this session
                        -------------------    ------------------
Existing test files             212                    212
test_regression.py                0                     25
test_edge_cases.py                0                     39
                              -----                  -----
Total passing                   212                    271
xfail (known open bugs)           0                      5
Failures                          0                      0
