ronantakizawa/codereview-bench huggingface dataset:
Repository: FlowiseAI/Flowise
File: packages/server/src/enterprise/utils/authSecrets.ts (TypeScript)

Code before:
import { getOrCreateStoredSecret } from '../../utils'

/**
 * Weak default values that were previously hardcoded when env vars were not set.
 * If the user has set a var to one of these, we treat it as "not set" and use file/AWS storage instead.
 */
const WEAK_DEFAULTS: Record<string, string> = {
    JWT_AUTH_TOKEN_SECRET: 'AABBCCDDAABBCCDDAABBCCDDAABBCCDDAABBCCDD',
    JWT_REFRESH_TOKEN_SECRET: 'AABBCCDDAABBCCDDAABBCCDDAABBCCDDAABBCCDD'
}

let tokenHashSecret: string | undefined
let expressSessionSecret: string | undefined
let jwtAuthTokenSecret: string | undefined
let jwtRefreshTokenSecret: string | undefined
let jwtIssuer: string | undefined
let jwtAudience: string | undefined

const NOT_INITIALIZED = 'Auth secrets not initialized. Call initAuthSecrets() first.'

/**
 * Initialize auth secrets from env (backwards compat) → AWS Secrets Manager → filesystem.
 * Each secret is generated with crypto.randomBytes(32) when created (or 'flowise' for JWT_ISSUER/JWT_AUDIENCE).
 * Call once after getEncryptionKey() in initDatabase().
 */
export async function initAuthSecrets(): Promise<void> {
    tokenHashSecret = await getOrCreateStoredSecret({
        envKey: 'TOKEN_HASH_SECRET',
        fileName: 'token_hash_secret.key',
        awsSecretIdSuffix: 'TokenHashSecret'
    })

    expressSessionSecret = await getOrCreateStoredSecret({
        envKey: 'EXPRESS_SESSION_SECRET',

Review comment (security):
Is there a reason we’re not including `EXPRESS_SESSION_SECRET` and `TOKEN_HASH_SECRET` in this configuration?
```ts
    EXPRESS_SESSION_SECRET: 'flowise'
    TOKEN_HASH_SECRET: 'popcorn'
```


Code after:
import { getOrCreateStoredSecret } from '../../utils'

/**
 * Weak default values that were previously hardcoded when env vars were not set.
 * If the user has set a var to one of these, we treat it as "not set" and use file/AWS storage instead.
 */
const WEAK_DEFAULTS: Record<string, string> = {
    JWT_AUTH_TOKEN_SECRET: 'AABBCCDDAABBCCDDAABBCCDDAABBCCDDAABBCCDD',
    JWT_REFRESH_TOKEN_SECRET: 'AABBCCDDAABBCCDDAABBCCDDAABBCCDDAABBCCDD',
    EXPRESS_SESSION_SECRET: 'flowise',
    TOKEN_HASH_SECRET: 'popcorn'
}

let tokenHashSecret: string | undefined
let expressSessionSecret: string | undefined
let jwtAuthTokenSecret: string | undefined
let jwtRefreshTokenSecret: string | undefined
let jwtIssuer: string | undefined
let jwtAudience: string | undefined

const NOT_INITIALIZED = 'Auth secrets not initialized. Call initAuthSecrets() first.'

/**
 * Initialize auth secrets from env (backwards compat) → AWS Secrets Manager → filesystem.
 * Each secret is generated with crypto.randomBytes(32) when created (or 'flowise' for JWT_ISSUER/JWT_AUDIENCE).
 * Call once after getEncryptionKey() in initDatabase().
 */
export async function initAuthSecrets(): Promise<void> {
    tokenHashSecret = await getOrCreateStoredSecret({
        envKey: 'TOKEN_HASH_SECRET',
        fileName: 'token_hash_secret.key',
        awsSecretIdSuffix: 'TokenHashSecret',
        weakDefault: WEAK_DEFAULTS.TOKEN_HASH_SECRET
    })
Repository: FreeCAD/FreeCAD
File: src/Gui/Selection/SelectionObserverPython.cpp (C++)

Code before:
    }
    catch (Py::Exception&) {
        Base::PyException e; // extract the Python error text
        e.ReportException();
    }
}


SelectionObserverPython::SelectionObserverPython(const Py::Object& obj, ResolveMode resolve)
    : SelectionObserver(true, resolve)
{
    this->init(obj.ptr());
}

SelectionObserverPython::~SelectionObserverPython() = default;

void SelectionObserverPython::addObserver(const Py::Object& obj, ResolveMode resolve)
{
    _instances.push_back(new SelectionObserverPython(obj, resolve));
}

void SelectionObserverPython::removeObserver(const Py::Object& obj)
{
    SelectionObserverPython* obs=nullptr;
    for (std::vector<SelectionObserverPython*>::iterator it =
        _instances.begin(); it != _instances.end(); ++it) {
        if ((*it)->inst == obj) {
            obs = *it;
            _instances.erase(it);
            break;
        }
    }

    delete obs;
}

void SelectionObserverPython::onSelectionChanged(const SelectionChanges& msg)
{
    handleSelectionChanged(msg);
}


Review comment (suggestion):
```suggestion
    for (auto it = _instances.begin(); it != _instances.end(); ++it) {
```

Code after:
    }
    catch (Py::Exception&) {
        Base::PyException e; // extract the Python error text
        e.ReportException();
    }
}


SelectionObserverPython::SelectionObserverPython(const Py::Object& obj, ResolveMode resolve)
    : SelectionObserver(true, resolve)
{
    this->init(obj.ptr());
}

SelectionObserverPython::~SelectionObserverPython() = default;

void SelectionObserverPython::addObserver(const Py::Object& obj, ResolveMode resolve)
{
    _instances.push_back(new SelectionObserverPython(obj, resolve));
}

void SelectionObserverPython::removeObserver(const Py::Object& obj)
{
    SelectionObserverPython* obs=nullptr;
    for (auto it =_instances.begin(); it != _instances.end(); ++it) {
        if ((*it)->inst == obj) {
            obs = *it;
            _instances.erase(it);
            break;
        }
    }

    delete obs;
}

void SelectionObserverPython::onSelectionChanged(const SelectionChanges& msg)
{
    handleSelectionChanged(msg);
}
Repository: FreeCAD/FreeCAD
File: src/Gui/Selection/Selection.cpp (C++)

Code before:
                    temp.back()._SubNameSet.insert(subelement);
            }
            SortMap.insert(std::make_pair(obj,temp.size()-1));
        }
    }

    return temp;
}

bool SelectionSingleton::needPickedList() const
{
    return _needPickedList;
}

void SelectionSingleton::enablePickedList(bool enable)
{
    if(enable != _needPickedList) {
        _needPickedList = enable;
        _PickedList.clear();
        notify(SelectionChanges(SelectionChanges::PickedListChanged));
    }
}

static void notifyDocumentObjectViewProvider(const SelectionChanges& changes) {
    const auto* doc = App::GetApplication().getDocument(changes.pDocName);
    if (!doc) return;

    const auto* obj = doc->getObject(changes.pObjectName);
    if (!obj) return;

    auto* vp = Application::Instance->getViewProvider(obj);
    if (!vp) return;

    vp->onSelectionChanged(changes);
}

void SelectionSingleton::notify(SelectionChanges &&Chng)
{
    if(Notifying) {
        NotificationQueue.push_back(std::move(Chng));
        return;
    }
    Base::FlagToggler<bool> flag(Notifying);
    NotificationQueue.push_back(std::move(Chng));
    while(!NotificationQueue.empty()) {
        const auto &msg = NotificationQueue.front();
        bool notify;
        switch(msg.Type) {
        case SelectionChanges::AddSelection:
            notify = isSelected(msg.pDocName, msg.pObjectName, msg.pSubName, ResolveMode::NoResolve);
            break;

Review comment (suggestion):
our code style requires `{ }` even for one liners
```suggestion
    if (!doc) {
        return;
    }
```

Code after:
                    temp.back()._SubNameSet.insert(subelement);
            }
            SortMap.insert(std::make_pair(obj,temp.size()-1));
        }
    }

    return temp;
}

bool SelectionSingleton::needPickedList() const
{
    return _needPickedList;
}

void SelectionSingleton::enablePickedList(bool enable)
{
    if(enable != _needPickedList) {
        _needPickedList = enable;
        _PickedList.clear();
        notify(SelectionChanges(SelectionChanges::PickedListChanged));
    }
}

static void notifyDocumentObjectViewProvider(const SelectionChanges& changes) {
    const auto* doc = App::GetApplication().getDocument(changes.pDocName);
    if (!doc) {
        return;
    }

    const auto* obj = doc->getObject(changes.pObjectName);
    if (!obj) {
        return;
    }

    auto* vp = Application::Instance->getViewProvider(obj);
    if (!vp) {
        return;
    }

    vp->onSelectionChanged(changes);
}

void SelectionSingleton::notify(SelectionChanges &&Chng)
{
    if(Notifying) {
        NotificationQueue.push_back(std::move(Chng));
        return;
    }
    Base::FlagToggler<bool> flag(Notifying);
    NotificationQueue.push_back(std::move(Chng));
    while(!NotificationQueue.empty()) {
Repository: FreeCAD/FreeCAD
File: src/Gui/Selection/Selection.cpp (C++)

Code before:
        }
    }

    return temp;
}

bool SelectionSingleton::needPickedList() const
{
    return _needPickedList;
}

void SelectionSingleton::enablePickedList(bool enable)
{
    if(enable != _needPickedList) {
        _needPickedList = enable;
        _PickedList.clear();
        notify(SelectionChanges(SelectionChanges::PickedListChanged));
    }
}

static void notifyDocumentObjectViewProvider(const SelectionChanges& changes) {
    const auto* doc = App::GetApplication().getDocument(changes.pDocName);
    if (!doc) return;

    const auto* obj = doc->getObject(changes.pObjectName);
    if (!obj) return;

    auto* vp = Application::Instance->getViewProvider(obj);
    if (!vp) return;

    vp->onSelectionChanged(changes);
}

void SelectionSingleton::notify(SelectionChanges &&Chng)
{
    if(Notifying) {
        NotificationQueue.push_back(std::move(Chng));
        return;
    }
    Base::FlagToggler<bool> flag(Notifying);
    NotificationQueue.push_back(std::move(Chng));
    while(!NotificationQueue.empty()) {
        const auto &msg = NotificationQueue.front();
        bool notify;
        switch(msg.Type) {
        case SelectionChanges::AddSelection:
            notify = isSelected(msg.pDocName, msg.pObjectName, msg.pSubName, ResolveMode::NoResolve);
            break;
        case SelectionChanges::RmvSelection:
            notify = !isSelected(msg.pDocName, msg.pObjectName, msg.pSubName, ResolveMode::NoResolve);
            break;

Review comment (suggestion):
```suggestion
    if (!obj) {
        return;
    }
```

Code after:
        }
    }

    return temp;
}

bool SelectionSingleton::needPickedList() const
{
    return _needPickedList;
}

void SelectionSingleton::enablePickedList(bool enable)
{
    if(enable != _needPickedList) {
        _needPickedList = enable;
        _PickedList.clear();
        notify(SelectionChanges(SelectionChanges::PickedListChanged));
    }
}

static void notifyDocumentObjectViewProvider(const SelectionChanges& changes) {
    const auto* doc = App::GetApplication().getDocument(changes.pDocName);
    if (!doc) {
        return;
    }

    const auto* obj = doc->getObject(changes.pObjectName);
    if (!obj) {
        return;
    }

    auto* vp = Application::Instance->getViewProvider(obj);
    if (!vp) {
        return;
    }

    vp->onSelectionChanged(changes);
}

void SelectionSingleton::notify(SelectionChanges &&Chng)
{
    if(Notifying) {
        NotificationQueue.push_back(std::move(Chng));
        return;
    }
    Base::FlagToggler<bool> flag(Notifying);
    NotificationQueue.push_back(std::move(Chng));
    while(!NotificationQueue.empty()) {
        const auto &msg = NotificationQueue.front();
        bool notify = false;
        switch(msg.Type) {
Repository: FreeCAD/FreeCAD
File: src/App/Datums.cpp (C++)

Code before:
PROPERTY_SOURCE(App::LocalCoordinateSystem, App::GeoFeature)

DatumElement::DatumElement(bool hideRole)
    : baseDir{0.0, 0.0, 1.0}
{
    ADD_PROPERTY_TYPE(Role,
                      (""),
                      0,
                      App::Prop_ReadOnly,
                      "Role of the datum in the local coordinate system.");

    // The role is hidden by default. It is visible only when the datum is in a LCS
    if (hideRole) {
        Role.setStatus(Property::Hidden, true);
    }
}

DatumElement::~DatumElement() = default;

bool DatumElement::getCameraAlignmentDirection(Base::Vector3d& directionZ, Base::Vector3d& directionX, const char* subname) const
{
    Q_UNUSED(subname);
    
    Placement.getValue().getRotation().multVec(baseDir, directionZ);

    if (baseDir == Base::Vector3d(0, 0, 1)) {
        Placement.getValue().getRotation().multVec(Base::Vector3d(1., 0., 0.), directionX);
    }

    return true;
}

App::LocalCoordinateSystem* DatumElement::getLCS() const
{
    auto inList = getInList();
    for (auto* obj : inList) {
        auto* lcs = dynamic_cast<App::LocalCoordinateSystem*>(obj);
        if (lcs) {
            return lcs;
        }
    }

    return nullptr;
}

bool DatumElement::isOriginFeature() const
{
    const auto* lcs = getLCS();
    return lcs ? lcs->isOrigin() : false;
}


Review comment (suggestion):
Maybe relevant to
```suggestion
    Base::Vector3d Horizontal(0, 0, 1);
    if (baseDir == Horizontal) {
```

Code after:
PROPERTY_SOURCE(App::LocalCoordinateSystem, App::GeoFeature)

DatumElement::DatumElement(bool hideRole)
    : baseDir{0.0, 0.0, 1.0}
{
    ADD_PROPERTY_TYPE(Role,
                      (""),
                      0,
                      App::Prop_ReadOnly,
                      "Role of the datum in the local coordinate system.");

    // The role is hidden by default. It is visible only when the datum is in a LCS
    if (hideRole) {
        Role.setStatus(Property::Hidden, true);
    }
}

DatumElement::~DatumElement() = default;

bool DatumElement::getCameraAlignmentDirection(Base::Vector3d& directionZ, Base::Vector3d& directionX, const char* subname) const
{
    Q_UNUSED(subname);
    
    Placement.getValue().getRotation().multVec(baseDir, directionZ);

    if (baseDir == Base::Vector3d::UnitZ) {
        Placement.getValue().getRotation().multVec(Base::Vector3d::UnitX, directionX);
    }

    return true;
}

App::LocalCoordinateSystem* DatumElement::getLCS() const
{
    auto inList = getInList();
    for (auto* obj : inList) {
        auto* lcs = dynamic_cast<App::LocalCoordinateSystem*>(obj);
        if (lcs) {
            return lcs;
        }
    }

    return nullptr;
}

bool DatumElement::isOriginFeature() const
{
    const auto* lcs = getLCS();
    return lcs ? lcs->isOrigin() : false;
}
Repository: FreeCAD/FreeCAD
File: src/Mod/Material/App/Array3DPyImp.cpp (C++)

Code before:
            PyErr_SetString(PyExc_IndexError, "Invalid array index");
            return nullptr;
        }
        Py_Return;
    }

    PyErr_SetString(PyExc_TypeError, "Expected (integer, string) arguments");
    return nullptr;
}

PyObject* Array3DPy::setValue(PyObject* args)
{
    int depth;
    int row;
    int column;
    PyObject* valueObj;
    if (PyArg_ParseTuple(args, "iiiO!", &depth, &row, &column, &PyUnicode_Type, &valueObj)) {
        Py::String item(valueObj);
        try {
            auto quantity = Base::Quantity::parse(item.as_string());
            quantity.setFormat(MaterialValue::getQuantityFormat());
            getArray3DPtr()->setValue(depth, row, column, quantity);
        }
        catch (const InvalidIndex&) {
            PyErr_SetString(PyExc_IndexError, "Invalid array index");
            return nullptr;
        }
        Py_Return;
    }

    PyErr_SetString(PyExc_TypeError, "Expected (integer, integer, integer, string) arguments");
    return nullptr;
}

PyObject* Array3DPy::setRows(PyObject* args)
{
    int depth;
    int rows;
    if (!PyArg_ParseTuple(args, "ii", &depth, &rows)) {
        return nullptr;
    }

    getArray3DPtr()->setRows(depth, rows);
    Py_Return;
}

PyObject* Array3DPy::getCustomAttributes(const char* /*attr*/) const
{
    return nullptr;
}


Review comment (bug):
```suggestion
            getArray3DPtr()->setValue(depth, row, column, quantity);
        }
        catch (const ParseError&) {
            PyErr_SetString(PyExc_ValueError, "Cannot parse value");
            return nullptr;
```

Code after:
            PyErr_SetString(PyExc_ValueError, e.what());
            return nullptr;
        }
        catch (const InvalidIndex& e) {
            PyErr_SetString(PyExc_IndexError, e.what());
            return nullptr;
        }
        Py_Return;
    }

    PyErr_SetString(PyExc_TypeError, "Expected (integer, string) arguments");
    return nullptr;
}

PyObject* Array3DPy::setValue(PyObject* args)
{
    int depth;
    int row;
    int column;
    PyObject* valueObj;
    if (PyArg_ParseTuple(args, "iiiO!", &depth, &row, &column, &PyUnicode_Type, &valueObj)) {
        Py::String item(valueObj);
        try {
            auto quantity = Base::Quantity::parse(item.as_string());
            quantity.setFormat(MaterialValue::getQuantityFormat());
            getArray3DPtr()->setValue(depth, row, column, quantity);
        }
        catch (const Base::ParserError& e) {
            PyErr_SetString(PyExc_ValueError, e.what());
            return nullptr;
        }
        catch (const InvalidIndex& e) {
            PyErr_SetString(PyExc_IndexError, e.what());
            return nullptr;
        }
        Py_Return;
    }

    PyErr_SetString(PyExc_TypeError, "Expected (integer, integer, integer, string) arguments");
    return nullptr;
}

PyObject* Array3DPy::setRows(PyObject* args)
{
    int depth;
    int rows;
    if (!PyArg_ParseTuple(args, "ii", &depth, &rows)) {
        return nullptr;
    }

    getArray3DPtr()->setRows(depth, rows);
Repository: FreeCAD/FreeCAD
File: src/Gui/Dialogs/DlgParameterFind.cpp (C++)

Code before:

        // if search was successful then make it the current item
        if (next) {
            groupTree->setCurrentItem(next);
        }
        else {
            int ret = QMessageBox::warning(this, tr("Not found"), tr("%1 not found. Would you like to start from the beginning?").arg(opt.text),
                QMessageBox::Yes| QMessageBox::No);

            switch (ret) {
                case QMessageBox::No:
                    break;
                case QMessageBox::Yes:
                    opt.text = "BaseApp";
                    opt.group = ui->checkGroups->isChecked();
                    opt.name = ui->checkNames->isChecked();
                    opt.value = ui->checkValues->isChecked();
                    opt.match = ui->checkMatch->isChecked();

                    groupTree->setCurrentItem(top);
                    DlgParameterFind::findItem(current, opt);
                    break;
                default:
                    // should never be reached
                    break;
                }
            
        }
    }
}

void DlgParameterFind::reject()
{
    QDialog::reject();
}

#include "moc_DlgParameterFind.cpp"


Review comment (suggestion):
```suggestion
            auto ret = QMessageBox::warning(this, tr("Not found"), tr("%1 not found. Would you like to start from the beginning?").arg(opt.text),
                QMessageBox::Yes| QMessageBox::No);
            if (ret == QMessageBox::Yes) {
                opt.text = "BaseApp";
                opt.group = ui->checkGroups->isChecked();
                opt.name = ui->checkNames->isChecked();
                opt.value = ui->checkValues->isChecked();
                opt.match = ui->checkMatch->isChecked();

                groupTree->setCurrentItem(top);
                DlgParameterFind::findItem(current, opt);
            }
```

Code after:
            }
        }

        // if search was successful then make it the current item
        if (next) {
            groupTree->setCurrentItem(next);
        }
        else {
            auto ret = QMessageBox::warning(
                this,
                tr("Not found"),
                tr("%1 not found. Would you like to start from the beginning?").arg(opt.text),
                QMessageBox::Yes | QMessageBox::No
            );
            if (ret == QMessageBox::Yes) {
                opt.text = "BaseApp";
                opt.group = ui->checkGroups->isChecked();
                opt.name = ui->checkNames->isChecked();
                opt.value = ui->checkValues->isChecked();
                opt.match = ui->checkMatch->isChecked();

                groupTree->setCurrentItem(top);
                DlgParameterFind::findItem(current, opt);
            }
        }
    }
}

void DlgParameterFind::reject()
{
    QDialog::reject();
}

#include "moc_DlgParameterFind.cpp"
Repository: FreeCAD/FreeCAD
File: src/Mod/TechDraw/Gui/QGIViewPart.cpp (C++)

Code before:
    if (!scenePage) {
        return false;
    }

    return scenePage->getExportingAny();
}


// returns true if vertex dots should be shown
bool QGIViewPart::showVertices() const
{
    // dvp already validated
    auto dvp(static_cast<TechDraw::DrawViewPart*>(getViewObject()));

    if (dvp->CoarseView.getValue()) {
        // never show vertices in CoarseView
        return false;
    }

    // if (isSelected()) {
    //     return true;
    // }

    // if we have selected verts?

    return true;
}


// returns true if arc center marks should be shown
bool QGIViewPart::showCenterMarks() const
{
    // dvp and vp already validated
    auto dvp(static_cast<TechDraw::DrawViewPart*>(getViewObject()));
    auto vp(static_cast<ViewProviderViewPart*>(getViewProvider(dvp)));

    if (isExporting() && Preferences::printCenterMarks()) {
        return true;
    }

    return vp->ArcCenterMarks.getValue();
}

//! true if center marks (type of vertex) should be hidden
bool QGIViewPart::hideCenterMarks() const
{
    // printing
    if (isExporting() &&
        Preferences::printCenterMarks()) {
        return false;
    }

Review comment (suggestion):
The "_Show center marks_" preference is not having any effect on the visibility of the center marks on the page. I think this function needs to be updates to check the `Preferences::showCenterMarks()`?

Also, I turned "_Print center marks_" to `false`, and the center marks still showed on the pdf when exported (tried with the "Export PDF" context menu command and the global "Export..." command in the File menu). 

I've tested the the PR for about 8 minutes trying to break anything and everything else seems to be working.

Code after:
{
    // dvp already validated
    auto viewPart {freecad_cast<TechDraw::DrawViewPart*>(getViewObject())};
    auto vpPage = getViewProviderPage(viewPart);

    QGSPage* scenePage = vpPage->getQGSPage();
    if (!scenePage) {
        return false;
    }

    return scenePage->getExportingAny();
}


// returns true if vertex dots should be shown
// note this is only one of the "rules" around showing or hiding vertices.
bool QGIViewPart::showVertices() const
{
    // dvp already validated
    auto dvp(static_cast<TechDraw::DrawViewPart*>(getViewObject()));
    return !dvp->CoarseView.getValue();
}


// returns true if arc center marks should be shown
bool QGIViewPart::showCenterMarks() const
{
    // dvp and vp already validated
    auto dvp(static_cast<TechDraw::DrawViewPart*>(getViewObject()));
    auto vp(static_cast<ViewProviderViewPart*>(getViewProvider(dvp)));

    if (isExporting() && Preferences::printCenterMarks()) {
        return true;
    }

    return vp->ArcCenterMarks.getValue();
}

//! true if center marks (type of vertex) should be hidden
bool QGIViewPart::hideCenterMarks() const
{
    // printing
    if (isExporting() &&
        Preferences::printCenterMarks()) {
        return false;
    }

    // on screen
    if (showCenterMarks()) {
        return false;
    }